Skip to content

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

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

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

Conversation

@zulquer

@zulquer zulquer commented Jul 7, 2026

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

Copilot AI 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.

File Description
src/multisafepay/client/client.py Prevents referencing response when transport fails before returning one; wraps 5xx failures as ApiException.
tests/multisafepay/unit/client/test_unit_client.py Adds 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")

@danielcivit danielcivit 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

Sign up for free to 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