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
Middleware is executed incorrectly when using fastify #11585
Comments
I'm not sure but this might be related to something we already discussed here #1628 |
@kamilmysliwiec, I have looked into #1628 and came to the conclusion that while they both deal with middleware executing unexpectedly, both the cause and reported behavior differ from this issue. The differences
Some good newsI tested the routes as reported in #1628 and the changes I made in my own Already reported beforeIt would seem like this Fastify specific issue was reported before in #8431 but it was, in my opinion, wrongfully closed as a duplicate of #1628. As mentioned, the cause is different from #1628. |
If you look at my response here #8431 (comment), you'll see that we were aware of this issue, and the reason it was closed as a duplicate of #1628 is that they both boil down to the same discussion - whether or not middleware should be registered as middleware (as the name suggests), or a route handler (express) or a hook (fastify). |
By going through #8431, #1628, and #9809 I was unable to conclude that they discuss the usage of route handlers and hooks. There is no mention of either. That being said, I want to summarize what I have learned looking into this so that we can get on the same page. For context, controllers and their routes:
(This setup is available to test and play around with at my reproduction) In the following table you can see the amount of time middleware is executed when fetching the path in the first column.
The only time the Express handler is acting in an unexpected way is with the problem reported in #1628 where a path is matched by both The Fastify handler on the other hand has many more failures. This is what is reported in this issue and #8431. As I stated before, the root cause of this is usage of Middie which does not add middleware to a specific route, but rather, a prefix. I hope this clarifies any confusion. |
Thanks for providing a detailed description/summary of this issue! If you could submit a PR so we can move the convo there that would be great 🙌 |
@kamilmysliwiec @MatthiasKunnen any update on this issue? I'm trying to register a middleware for a prefixed route. But I'm experiencing exactly this issue. I'm constantly getting |
Let's track this here #11718 |
@DennisSnijder, I'm still working on a solution. The work by @kamilmysliwiec in #11718 has already improved the adapter significantly, see the following before and after screenshot. I'm still hoping to be able to contribute a PR that completely addresses this issue. The approach I'm working on is a bit more intricate from @kamilmysliwiec's but should be more performant by avoiding an extra regex execution and function for every request. It will probably be a while before I can finalize this as I'm trying to coordinate an extra feature to path-to-regexp to accomplish the envisioned implementation. |
Is there an existing issue for this?
Current behavior
Middleware is executed on routes it was not applied to
Registering middleware on a controller that has a
/
route, will lead to that middleware executing on all routes regardless of controller. This affects all subroutes of a route with middleware.Another example: register middleware on
/auth
and have another route in another controller,/auth/login
. Now fetch/auth/login
, the middleware that is applied to/auth
will be executed on/auth/login
.Middleware is executed multiple times for the same route
If a controller that has middleware applied has n routes that are prefixes of each other, the middleware will be called n times for the most specific URL.
For example, suppose there are three routes in the controller;
/a
,/a/b
, and/a/b/c
. Fetching the last route will execute the middleware three times.Minimum reproduction code
https://github.com/MatthiasKunnen/nest-fastify-middleware-hooks-demo
Steps to reproduce
No response
Expected behavior
Middleware should execute only once and only for the routes of the controller it has been applied to.
Package
@nestjs/platform-fastify
Packages versions
In which operating systems have you tested?
Other
The reproduction's README contains more details.
The cause
The cause behind these issues seems to be a misunderstanding regarding how middie (fastify's middleware package) works. Middie does not add middleware to a specific route, but rather, a prefix. I originally made the same misunderstanding and thought the bug was a problem with Middie, however this behavior is intended according to a package author. See my original issue Middie issue here: fastify/middie#113.
A proposed solution
To address this bug, I've followed the recommendations received from Middie's author and replaced usage of Middie with hooks. See reproduction README for more details. I can create a PR with these change to discuss them in depth.
The text was updated successfully, but these errors were encountered: