PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

PTHMINT-131: Add error handling for transport exceptions and server e… - #68

Open
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131
Open

PTHMINT-131: Add error handling for transport exceptions and server e…#68
zulquer wants to merge 3 commits into
masterfrom
PTHMINT-131

Conversation

@zulquer

Copy link
Copy Markdown
Collaborator

This pull request improves error handling in the Client class and adds comprehensive unit tests to ensure correct behavior when network errors or server errors occur. The main changes focus on making exception handling more robust and well-tested.

Error handling improvements:

  • Updated _create_request in client.py to initialize response to None and check for response is not None before accessing its attributes, preventing errors when the transport fails before returning a response. [1][2]

Testing enhancements:

  • Added new test stubs (_FailingTransport, _ServerErrorTransport, _ServerErrorResponse) to simulate network failures and server errors, enabling more thorough testing of error handling logic in the Client.
  • Added a unit test to verify that exceptions raised by the transport before a response exists are reraised as-is.
  • Added a unit test to ensure that server error responses (HTTP 5xx) are wrapped and raised as an ApiException.
  • Imported ApiException in the test module to support the new tests.…rrors in Client

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Client._create_request error handling when the underlying transport fails before producing a response object, and adds unit tests to cover both “no response” transport failures and HTTP 5xx server-error wrapping into ApiException.

Changes:

  • Initialize response = None in _create_request and guard access to response.status_code to avoid errors when the transport raises before returning a response.
  • Add transport/response test stubs to simulate network failures and 5xx responses.
  • Add unit tests for re-raising transport exceptions without a response, and wrapping 5xx errors into ApiException.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/multisafepay/client/client.pyPrevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.pyAdds unit coverage for transport exceptions without a response and for 5xx server error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +378 to +379
with pytest.raises(ApiException, match="Request failed: server error"):
client.create_get_request("json/orders")

@danielcivitdanielcivit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing. Please check the details and part of these changes were already merged here: #65

@codecov

codecovBot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.79%. Comparing base (b6f3587) to head (c593288).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #68 +/- ##
==========================================
+ Coverage 92.67% 92.79% +0.11% 
==========================================
Files 182 182 Lines 3357 3357 ==========================================
+ Hits 3111 3115 +4 + Misses 246 242 -4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulquer
zulquer requested a review from danielcivitJuly 29, 2026 07:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@zulquer@danielcivit