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

Initial error handling #36

Merged
merged 11 commits into from
Dec 27, 2023
Merged

Initial error handling #36

merged 11 commits into from
Dec 27, 2023

Conversation

andrii-otroshko-forte
Copy link
Contributor

This PR contains initial error handling in the Fauna client:

  • Implemented the required error types as C# exceptions.
  • Updated the client to handle these errors according to the proposed hierarchy.
  • Added handling for HttpClient errors, converting them into Fauna exceptions with original errors as inner exceptions.
  • Updated related Client unit tests. If this implementation does not require refactoring, I plan to add more tests for extensive coverage

Copy link
Contributor

@pnwpedro pnwpedro left a comment

Choose a reason for hiding this comment

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

Good start, and the direction is right. I added a few specific comments, but generally what I'd like to see is more specificity on each type. Ideally minimize the number of nullable properties so users can confidently interact with error objects without reading documentation about what might be null. In addition, make sure they are elevated to the top level.

Copy link
Contributor

@pnwpedro pnwpedro left a comment

Choose a reason for hiding this comment

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

A few more things to sort out I think

Added initial backoff retry impementation
@andrii-otroshko-forte andrii-otroshko-forte merged commit 8224739 into main Dec 27, 2023
4 checks passed
@andrii-otroshko-forte andrii-otroshko-forte deleted the initial_error_handling branch December 27, 2023 08:04
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.

4 participants