-
Notifications
You must be signed in to change notification settings - Fork 6
JCL-402: RDF4J body handlers http error handling #1162
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
Conversation
bd71db4 to
1ef8bee
Compare
langsamu
left a comment
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.
Same comment as for the Jena proposal.
4b37262 to
39299b6
Compare
1ef8bee to
c82183a
Compare
rdf4j/src/main/java/com/inrupt/client/rdf4j/RDF4JBodyHandlers.java
Outdated
Show resolved
Hide resolved
39299b6 to
f92ed37
Compare
9501c47 to
452ce77
Compare
ffef3bc to
b8e402f
Compare
4fbd520 to
f534a65
Compare
b8e402f to
63be5e6
Compare
f534a65 to
c795127
Compare
bf051a1 to
d0a0003
Compare
rdf4j/src/main/java/com/inrupt/client/rdf4j/RDF4JBodyHandlers.java
Outdated
Show resolved
Hide resolved
| private static void throwOnError(final Response.ResponseInfo responseInfo) { | ||
| if (!Response.isSuccess(responseInfo.statusCode())) { | ||
| throw new ClientHttpException( | ||
| "Could not map to an RDF4J entity.", |
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.
The real issue is that the request failed rather been an issue mapping it to a RDF4J entity. Is this message a bit misleading?
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.
I see your point. Is 475adc0 better?
* Revert "JCL-402: RDF4J body handlers http error handling (#1162)" This reverts commit e03eb1e. * Revert "JCL-402: JSONB test coverage (#1184)" This reverts commit 52d485b. * Revert "JCL-402: HTTP error handling in JenaBodyHandlers (#1159)" This reverts commit 89534b4. * Move ProblemDetails to solid module, remove ClientHttpException * ProblemDetails to interface in API module
This is based on #1159 , which should be reviewed first. (The only dependency between the two is in the mocks)
This deprecates the current methods in
RDF4JBodyHandlers, and replaces them with similar method throwing an appropriate exception with error details on HTTP error instead of returning an empty dataset. This makes use of the composable throwing body handler now exposed by theResponseinterface of theapimodule. The new method now haveRDF4Jin their name for this not to be a breaking change:ofModelbecomesofRDF4JModel, etc.