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

Created a method to add a custom 404 error handler to router #801

Closed
wants to merge 3 commits into from

Conversation

slinkydeveloper
Copy link
Member

Added a method to Router interface to assign a custom 404 not found error handler

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

* @param handler the handler to run
* @return a reference to this, so the API can be used fluently
*/
Router setNotFoundHandler(Handler<RoutingContext> handler);
Copy link
Contributor

Choose a reason for hiding this comment

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

missing @Fluent annotation

Copy link
Contributor

Choose a reason for hiding this comment

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

name it instead notFoundHandler

Copy link
Contributor

Choose a reason for hiding this comment

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

@slinkydeveloper ping on this :-)

@@ -374,4 +374,12 @@ static Router router(Vertx vertx) {
*/
void handleFailure(RoutingContext context);

/**
* Set an error handler for 404 Not Found error
Copy link
Contributor

Choose a reason for hiding this comment

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

please document when the handler should be called

@vietj
Copy link
Contributor

vietj commented Jan 12, 2018

please avoid reformed code that is not modified (extra blank are added)

@slinkydeveloper
Copy link
Member Author

I've also updated doc inside package-info

@vietj
Copy link
Contributor

vietj commented Jan 16, 2018

this looks ok to merge, @pmlopes WDYT ?

@ufoscout
Copy link

Any chance this is going to be merged soon? We have a bug in production that depends on this PR :-(

@vietj
Copy link
Contributor

vietj commented May 15, 2018

@slinkydeveloper can you rebase ?
@pmlopes can you finish the review ?

Signed-off-by: francesco <francescoguard@gmail.com>
Signed-off-by: francesco <francescoguard@gmail.com>
Signed-off-by: francesco <francescoguard@gmail.com>
@slinkydeveloper
Copy link
Member Author

@vietj done, please check if I've done mistakes with new docs

@ufoscout
Copy link

ufoscout commented Jun 8, 2018

Is there a target milestone for this to be merged?

@ixtli
Copy link

ixtli commented Jan 14, 2019

Any updates on this? I've been searching for how to correctly register a default 404 handler for about an hour now (the only documentation is basically nonexistent https://vertx.io/docs/vertx-web/java/#_default_404_handling ) and router.rout().failureHandler(...) as discussed in other PRs/issues doesn't work at all as of 3.6.x.

@vietj
Copy link
Contributor

vietj commented Jan 14, 2019

@pmlopes any reason we can't merge this ?

@vietj vietj added this to the 3.6.3 milestone Jan 14, 2019
@vietj vietj mentioned this pull request Jan 14, 2019
24 tasks
@vietj vietj requested a review from pmlopes January 14, 2019 16:05
@vietj
Copy link
Contributor

vietj commented Jan 14, 2019

one reason is that there are un-resolve items in the PR

@slinkydeveloper
Copy link
Member Author

Should I rebase?

@vietj
Copy link
Contributor

vietj commented Jan 15, 2019

there does not eem to have conflicts, there are unanswered concerns I commented on a few monthgs ago

@vietj
Copy link
Contributor

vietj commented Jan 15, 2019

like rename setNotFoundHandler to notFoundHandler, etc...

@MeYoGui
Copy link
Contributor

MeYoGui commented Jan 23, 2019

Any updates on this? I've been searching for how to correctly register a default 404 handler for about an hour now (the only documentation is basically nonexistent https://vertx.io/docs/vertx-web/java/#_default_404_handling ) and router.rout().failureHandler(...) as discussed in other PRs/issues doesn't work at all as of 3.6.x.

Hi @ixtli. On my project we are registering an Handler to the router like this :

final NotFoundHandler notFoundHandler = new NotFoundHandler ();
router.route().last().handler(notFoundHandler);

Since our general handlers doesn't call "next" then the NotFoundHandler will only be called on route that are not going through the "general handler".

If a route is found then the normal handler is called and the NotFoundHandler is not called (as we don't call "next" on this handler).

It could be a workaround but you have to be careful on how you implement your "general handler".
Hope it helped.

@slinkydeveloper
Copy link
Member Author

This PR #1140 should definitely solve the error handling. Can you give feedback/partecipate to the discussion?

@vietj vietj modified the milestones: 3.6.3, 3.7.0 Jan 31, 2019
@slinkydeveloper
Copy link
Member Author

Solved by #1142

@slinkydeveloper slinkydeveloper deleted the custom_404 branch February 6, 2019 14:34
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

5 participants