Skip to content
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

Reorganized Router error handling #1140 #1142

Merged
merged 1 commit into from Jan 28, 2019
Merged

Reorganized Router error handling #1140 #1142

merged 1 commit into from Jan 28, 2019

Conversation

slinkydeveloper
Copy link
Member

Signed-off-by: francesco francescoguard@gmail.com

Signed-off-by: francesco <francescoguard@gmail.com>
@vietj
Copy link
Contributor

vietj commented Jan 24, 2019

I think it's fine for 3.6.3

This can then be handled by your own error handler, or perhaps the augmented error handler that we supply to use,
or if no error handler is provided Vert.x-Web will send back a basic 404 (Not Found) response.

A default 404 error handler is provided by Router. Anyway you can provide your own 404 error handler with
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for "Anyway", simply "You can provide..."

@@ -363,10 +363,24 @@ default void accept(HttpServerRequest request) {
*
* @param exceptionHandler the exception handler
* @return a reference to this, so the API can be used fluently
* @deprecated you should use {@link Router#errorHandler(int, Handler)} with 500 status code
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to remove that in Vert.x 4 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes this is the idea. Maybe we can put the new method in 3.7 and remove the old one in 4

Copy link
Member Author

Choose a reason for hiding this comment

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

And of course I will reflect these changes in vertx-web-api-contract too

@@ -272,9 +273,19 @@ public Router mountSubRouter(String mountPoint, Router subRouter) {
return this;
}

@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to deprecate the implementation when the base method is deprecated

@slinkydeveloper slinkydeveloper modified the milestones: 4.0.0, 3.6.3 Jan 25, 2019
@pmlopes pmlopes merged commit 08139ca into master Jan 28, 2019
@vietj vietj mentioned this pull request Jan 28, 2019
24 tasks
pmlopes pushed a commit that referenced this pull request Jan 28, 2019
Signed-off-by: francesco <francescoguard@gmail.com>

(cherry picked from commit 08139ca)
@slinkydeveloper slinkydeveloper deleted the error_handler branch March 25, 2019 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants