failureHandler never called after upgrade to v3.5.0

[santo74]

Version

• vert.x core: 3.5.0
• vert.x web: 3.5.0

Context

After I upgraded from v3.4.1 to v3.5.0 I noticed that failureHandlers are never called.
Router is defined like this:

            Router router = Router.router(vertx);
            
            router.get("/").handler(ctx -> {
                ctx.response().putHeader("content-type", "text/plain").end("Welcome to the homepage!");
            });

            router.route().failureHandler(ctx -> {
                ctx.response().putHeader("content-type", "text/plain").end("Routing failure!");
            });

A call to http://localhost returns "Welcome to the homepage", but a call to e.g. http://localhost/test returns the default "Resource not found" instead of "Routing failure!"

Do you have a reproducer?

No

Steps to reproduce

Just set up a webserver with a router as explained in the example above and try to access the root ("/") and then "/test".
As a result the request to "/test" returns the default 404 message instead of the custom message defined in the failureHandler

Please note that the exact same code worked before the upgrade to Vert.x 3.5.0

Extra

/

[slinkydeveloper]

Maybe you are affected from this? #739

[santo74]

Thanks for the pointer to that issue.
However, I doubt that's the cause of my problem.
As I understand, that issue is related to concurrency while I experience the issue all the time, even when running a simple Webserver with simple routing (as per the code above) and firing a single request.

[vietj]

@slinkydeveloper can you investigate ? I'm going to scope it for 3.5.1

[slinkydeveloper]

I just tried to configure an empty project with version 3.4.1 with your code and It throws the default 404.
The failure handler is called when another handler throws a failure with routingContext.fail(). In vert.x router the 404 failure is never thrown by an handler, is created directly by the routing algorithm
Give a look at this method: https://github.com/vert-x3/vertx-web/blob/master/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java#L125

[santo74]

You're right, I'm afraid my example code above was not complete. Sorry about that!
Apparently there was also a StaticHandler involved:

            Router router = Router.router(vertx);
            
            router.get("/").handler(ctx -> {
                ctx.response().putHeader("content-type", "text/plain").end("Welcome to the homepage!");
            });

            router.route().handler(StaticHandler.create());

            router.route().failureHandler(ctx -> {
                ctx.response().putHeader("content-type", "text/plain").end("Routing failure!");
            });

And guess what? It's a change in that StaticHandler that explains the behaviour I'm experiencing.
More specifically this commit:
3a0005d#diff-1ebbcde17b4a2cabb90c51567ae716aa
Before the change a 404 was returned (context.fail()), which triggered my failureHandler.
After the change, context.next() is called, which doesn't trigger the failureHandler

So the solution in my case is to not rely on a failure in the StaticHandler but instead specify another catch-all route that will be triggered when the requested file can't be served by the StaticHandler.

[slinkydeveloper]

If you are interested I've created a PR that allows you to create an handler that manages 404 error #801

[tsegismont]

@pmlopes shouldn't the new behavior of StaticHandler be added to https://github.com/vert-x3/wiki/wiki/3.5.0-Breaking-Changes#vertx-web ?

[santo74]

@slinkydeveloper sounds interesting
@tsegismont Before an upgrade I always look at the breaking changes for that release to get an idea of the impact of the upgrade, so yes I think this definitely belongs in that document

[vietj]

what's the status of this issue ?

[santo74]

I think it's just a matter of clearly documenting the new behaviour in the breaking-changes document.
The PR #801 for a custom 404 handler might be a welcome addition, but not a blocker for this issue.

[vietj]

@slinkydeveloper can you take care of providing a PR for this as it is related to #801 ?

[slinkydeveloper]

I think that beahviour added by 3a0005d#diff-1ebbcde17b4a2cabb90c51567ae716aa is right. For example let's say you have a router like:

router.route(StaticHandler.create());
router.route("/api/hello", anHandler); 

And a webroot dir like:

/webroot:
  /api:
    /index.html

In this situation, without 3a0005d#diff-1ebbcde17b4a2cabb90c51567ae716aa the router will throw unexpected 404 when you do a request to /api/hello

I'm going to update the doc describing this behaviour.

[vietj]

thanks