Skip to content

Conversation

hantsy
Copy link
Contributor

@hantsy hantsy commented Nov 12, 2019

Mentioned in issue #4042, added sections for exception handlers. This PR should be clean finally when #4042 and #5374 are resolved.

@machi1990 machi1990 requested a review from geoand November 12, 2019 07:50
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@geoand
Copy link
Contributor

geoand commented Nov 12, 2019

Can you please squash into a single commit? Best not to have superfluous commits in the history

@hantsy hantsy force-pushed the improve-spring-exception-handler-docs branch from bd20ac8 to 4e7fab2 Compare November 12, 2019 08:22
@cescoffier cescoffier added area/documentation triage/waiting-for-ci Ready to merge when CI successfully finishes labels Nov 12, 2019
@cescoffier
Copy link
Member

@geoand should it be backported?

@geoand
Copy link
Contributor

geoand commented Nov 12, 2019

@cescoffier yes yes!

@cescoffier
Copy link
Member

I've restarted the failing tests as they look unrelated.


* `org.springframework.http.ResponseEntity`
* `java.util.Map`
* `void`, but in Quarkus, it must be combined with a `@ResponseStatus` annotation on the method to send an status error to HTTP response.
Copy link
Member

Choose a reason for hiding this comment

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

@geoand I think this is not true anymore?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, this restriction has been lifted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I notice the void item has been removed. If the void return type is supported, I think we need a void in the list. I know the issue is fixed.


=== Exception handler method parameter types

The following parameter types are supportted, in arbitrary order:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The following parameter types are supportted, in arbitrary order:
The following parameter types are supported, in arbitrary order:


The following parameter types are supportted, in arbitrary order:

* An exception argument: declared as a general Exception or as a more specific exception. This also serves as a mapping hint if the annotation itself does not narrow the exception types through its `value()`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* An exception argument: declared as a general Exception or as a more specific exception. This also serves as a mapping hint if the annotation itself does not narrow the exception types through its `value()`.
* An exception argument: declared as a general `Exception` or as a more specific exception. This also serves as a mapping hint if the annotation itself does not narrow the exception types through its `value()`.

The following parameter types are supportted, in arbitrary order:

* An exception argument: declared as a general Exception or as a more specific exception. This also serves as a mapping hint if the annotation itself does not narrow the exception types through its `value()`.
* Request and/or response objects (typically from the Servlet API). You may choose any specific request/response type, e.g. `ServletRequest` / `HttpServletRequest`. To use Servlet API, you should add `qarkus-undertow` dependency.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Request and/or response objects (typically from the Servlet API). You may choose any specific request/response type, e.g. `ServletRequest` / `HttpServletRequest`. To use Servlet API, you should add `qarkus-undertow` dependency.
* Request and/or response objects (typically from the Servlet API). You may choose any specific request/response type, e.g. `ServletRequest` / `HttpServletRequest`. To use Servlet API, you should add `quarkus-undertow` dependency.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

This is not ready yet. I added a couple of comments and questions.

@geoand geoand force-pushed the improve-spring-exception-handler-docs branch from 4e7fab2 to 9b5a8a9 Compare November 14, 2019 16:21
@geoand
Copy link
Contributor

geoand commented Nov 14, 2019

@gsmet I force pushed to the PR to address the issues you mentioned

@gsmet gsmet merged commit a0cf972 into quarkusio:master Nov 14, 2019
@gsmet
Copy link
Member

gsmet commented Nov 14, 2019

I checked it manually to avoid having to wait for CI. Will backport now.

@geoand
Copy link
Contributor

geoand commented Nov 14, 2019

Thanks @gsmet once again!!

@gsmet gsmet added this to the 1.0.0.Final milestone Nov 14, 2019
@gsmet gsmet removed the backport? label Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation triage/waiting-for-ci Ready to merge when CI successfully finishes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants