Expose failure reason in ElideResponse #690
Conversation
clayreimann
commented
Aug 8, 2018
clayreimann
commented
- Provide hook for building custom responses in JsonApiEndpoint
- Provide hook for building custom responses in JsonApiEndpoint
| @@ -121,7 +121,10 @@ public Response delete( | |||
| return build(elide.delete(path, jsonApiDocument, getUser.apply(securityContext))); | |||
| } | |||
|
|
|||
| private static Response build(ElideResponse response) { | |||
| return Response.status(response.getResponseCode()).entity(response.getBody()).build(); | |||
| protected Response build(ElideResponse response) { | |||
There was a problem hiding this comment.
Where does the failureReason get used? I see it being set, but never logged or added to a (presumably verbose?) response.
There was a problem hiding this comment.
Or is the plan that arbitrary endpoints could leverage this functionality?
There was a problem hiding this comment.
We'll be using it internally. Right now error cause is opaque to consumers of Elide unless you override a bunch of stuff.
| @@ -212,7 +212,7 @@ protected ElideResponse handleRequest(boolean isReadOnly, Object opaqueUser, | |||
| } | |||
| tx.flush(requestScope); | |||
|
|
|||
| ElideResponse response = buildResponse(responder.get()); | |||
| ElideResponse response = buildResponse(responder.get(), null); | |||
| } catch (JsonProcessingException e) { | ||
| return new ElideResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR, e.toString()); | ||
| return new ElideResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR, e.toString(), e); |
There was a problem hiding this comment.
There is no real analog to ElideResponse in GraphQL because we're leveraging GraphQLJava. The Elide class drives JsonAPI and is ignored by GraphQL, also the GraphQLEndpiont can't be customized in the way that JsonApiEndpoint can.
|
Would be nice to have a single test for both JSON-API and GraphQL. |
|
|
||
| /** | ||
| * Constructor. | ||
| * | ||
| * @param responseCode HTTP response code | ||
| * @param body returned body string | ||
| */ | ||
| public ElideResponse(int responseCode, String body) { | ||
| public ElideResponse(int responseCode, String body, Throwable failureReason) { |
There was a problem hiding this comment.
I would keep the original public constructor too and deprecate it.
clayreimann
force-pushed
the
add-failure-reason-to-ElideResponse
branch
from
0564449 to
c8ebfbf
Compare
