Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unexpected HTTP responses #94

Merged
merged 3 commits into from
Jun 28, 2024
Merged

Handle unexpected HTTP responses #94

merged 3 commits into from
Jun 28, 2024

Conversation

joshuaflanagan
Copy link
Contributor

@joshuaflanagan joshuaflanagan commented Jun 28, 2024

This will better handle the situation where the HTTP response body cannot be parsed as JSON.
Instead of throwing a JsonException, we will throw a ShipEngineException containing the HttpResponseMessage. This will give the caller a chance to handle the response in their own way.

We will also try to include a request ID on all ShipeEngineException now - by pulling from the response header in cases where we don't get a ShipEngine Error JSON response.

@coveralls
Copy link

coveralls commented Jun 28, 2024

Pull Request Test Coverage Report for Build 9717752223

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.9%) to 77.978%

Totals Coverage Status
Change from base Build 9714075367: 0.9%
Covered Lines: 654
Relevant Lines: 816

💛 - Coveralls

If the response cannot be parsed as JSON, we will throw a ShipEngineException, and include the actual response.
@coveralls
Copy link

coveralls commented Jun 28, 2024

Pull Request Test Coverage Report for Build 9718240694

Details

  • 18 of 18 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.3%) to 78.418%

Totals Coverage Status
Change from base Build 9714075367: 1.3%
Covered Lines: 654
Relevant Lines: 813

💛 - Coveralls

If error details weren't found, the user would get back an empty deserialized object.
Capture the request ID from the response headers, in case the body does not include it.
@joshuaflanagan joshuaflanagan marked this pull request as ready for review June 28, 2024 21:42
@coveralls
Copy link

coveralls commented Jun 28, 2024

Pull Request Test Coverage Report for Build 9718822717

Details

  • 32 of 32 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.3%) to 79.418%

Totals Coverage Status
Change from base Build 9714075367: 2.3%
Covered Lines: 657
Relevant Lines: 814

💛 - Coveralls

@joshuaflanagan joshuaflanagan merged commit 743d2d2 into main Jun 28, 2024
5 checks passed
@joshuaflanagan joshuaflanagan deleted the error-handling branch June 28, 2024 21:46
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