-
Notifications
You must be signed in to change notification settings - Fork 6
JCL-402: Refactor HTTP exception constructors #1161
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
JCL-402: Refactor HTTP exception constructors #1161
Conversation
Initial ClientHttpException class Create HTTP status enum Add some tests Add javadoc Refactor HttpStatus Add specialized builder to ProblemDetails Add missing header Make problem details parsing more resilient Add tests for problem details parsing The tests are in the solid-client module, because it will need the additional json service anyways in the test dependencies for testing the rest of this feature, and putting these tests in the api module creates a circular dependency. License header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not make more sense to move all the relevant fields and properties to the ClientHttpException?
It seems to belong there rather on this specialization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually these changes make this PR small enough that it can be bundled together with #1160
the JSON was invalid, causing the intended code path not to be used.
608e632 to
92358fa
Compare
The exception are now built taking a ProblemDetails in. Use HttpStatus from API module as reference
92358fa to
c41758e
Compare
|
I applied the changes here to #1160 |
8b193a9 to
71d6302
Compare
|
This is no longer required based on the refactoring of #1160 . |
The HTTP exception are now built taking a ProblemDetails in.