-
Notifications
You must be signed in to change notification settings - Fork 47
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
Exception Handling Section #137
base: staging
Are you sure you want to change the base?
Conversation
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.
-
src/main/java/io/openliberty/guides/rest/exceptions
should be created in the start directory with a.gitkeep
in it. - Add file hotspots for
GenericExceptionMapper.java
andNotFoundExceptionMapper.java
- Consider adding hotspots for
toResponse()
andErrorResponse
in the excerpt from the Exception Handling section below:
When JAX-RS sees any Throwable, it will search for a matching ExceptionMapper interface that has been marked with the @Provider annotation. It will pass the Throwable to the toResponse() method to construct a response using the ErrorResponse class.
@griffinhadfield changes made, hotspots added |
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.
Feedback has been addressed, LGTM.
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.
Suggested (via slack) describing how to set a Response
in a WebApplicationException
(or subclass) - and minor changes in wording.
README.adoc
Outdated
If a resource doesn't exist or the service experiences an exception, the client will receive either an empty response or an HTML error page respectively. | ||
A client should not receive either of these. | ||
Clients should receive JSON responses. |
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 would rephrase this section. I'm not sure that the client will always receive an empty response or an HTML error page. And ideally, the client would receive the error response in the same MediaType (MIME type) that they would receive a normal response in (i.e. it could be text, xml, etc.).
Maybe something like this:
Most application developers will want to ensure that when an exception occurs in the application (such as the client requesting a resource that does not exist or posting an invalid resource, etc.) that it is handled consistently and in a way that allows the client to fully understand the failure (i.e. bad input, server outage, etc.). This can be accomplished using the exception handling mechanisms in JAX-RS such as sending a Response
via a WebApplicationException
(or subclass) or by using ExceptionMapper
providers.
README.adoc
Outdated
A client should not receive either of these. | ||
Clients should receive JSON responses. | ||
|
||
This will be handled using an `ExceptionMapper` which maps exceptions to responses and builds appropriate JSON payloads. |
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.
This will be handled using an `ExceptionMapper` which maps exceptions to responses and builds appropriate JSON payloads. | |
The following example uses an `ExceptionMapper` which maps exceptions to responses and builds appropriate JSON payloads. |
README.adoc
Outdated
`src/main/java/io/openliberty/guides/rest/exceptions/NotFoundExceptionMapper.java` | ||
---- | ||
|
||
NotFoundExceptionMapper.java |
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.
We discussed in slack that it might be better to use a different exception type here - ideally an application-specific, business exception (like WidgetNotFoundException
) since NotFoundException
is a subclass of WebApplicationException
- and it could hold it's own Response
object (and thus doesn't need to be mapped).
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.
Just a couple tweaks - otherwise, looks good. Thanks!
README.adoc
Outdated
`src/main/java/io/openliberty/guides/rest/exceptions/NotFoundExceptionMapper.java` | ||
---- | ||
|
||
NotFoundExceptionMapper.java |
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.
NotFoundExceptionMapper.java | |
PropertyNotFoundExceptionMapper.java |
README.adoc
Outdated
[role="code_command hotspot file=2", subs="quotes"] | ||
---- | ||
#Create the `PropertyNotFoundExceptionMapper` class.# | ||
`src/main/java/io/openliberty/guides/rest/exceptions/NotFoundExceptionMapper.java` |
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.
`src/main/java/io/openliberty/guides/rest/exceptions/NotFoundExceptionMapper.java` | |
`src/main/java/io/openliberty/guides/rest/exceptions/PropertyNotFoundExceptionMapper.java` |
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.
Looks good! Thanks @AustinSeto!
Create section in guide to handle exceptions
Addresses #49