Middleware not applied when using setGlobalPrefix

[mentos1386]

trafficstars

Is there an existing issue for this?

• [X] I have searched the existing issues

Current behavior

I have an app which uses setGlobalPrefix('api') and then i have a module (non-root) which sets a middleware forRoutes('*').

I would expect the middleware to act on all routes. But it doesn't.

Minimum reproduction code

https://github.com/mentos1386/nestjs-observability/tree/e84c0b92be9416c0d29255007f3a107f6d0d900c

Steps to reproduce

1. npm ci
2. npx nx serve example
3. curl http://localhost:3000/api
4. Observe that there is no "YOU SHOULD SEE ME" in console.
5. Edit example/src/main.ts and comment out line 17.
6. curl http://localhost:3000
7. Observe that there is now "YOU SHOULD SEE ME" in console.

Expected behavior

I would expect the middleware to act on all routes. But it doesn't.

Package

• [ ] I don't know. Or some 3rd-party package
• [ ] @nestjs/common
• [X] @nestjs/core
• [ ] @nestjs/microservices
• [ ] @nestjs/platform-express
• [ ] @nestjs/platform-fastify
• [ ] @nestjs/platform-socket.io
• [ ] @nestjs/platform-ws
• [ ] @nestjs/testing
• [ ] @nestjs/websockets
• [ ] Other (see below)

Other package

No response

NestJS version

9.4.0

Packages versions

[System Information]
OS Version     : Linux 6.3
NodeJS Version : v20.0.0
NPM Version    : 8.19.4

[Nest CLI]
Nest CLI Version : 9.4.2

[Nest Platform Information]
platform-express version : 9.4.0
schematics version       : 9.1.0
testing version          : 9.4.0
common version           : 9.4.0
core version             : 9.4.0

Node.js version

20.0.0

In which operating systems have you tested?

• [ ] macOS
• [ ] Windows
• [X] Linux

Other

No response

[linkFly6]

I also encountered this problem. Has this problem solved?

[micalevisk]

@linkFly6 this GH Issue is still open, so no.

I didn't investigate it further yet. Feel free to do so.

[kamilmysliwiec]

Please provide a minimum reproduction repository (Git repository/StackBlitz/CodeSandbox project).

[mentos1386]

@kamilmysliwiec it's in the PR description. Is it not minimal enough?

[kamilmysliwiec]

Description != minimum reproduction repository

[mentos1386]

I meant the following:

Minimum reproduction code:

https://github.com/mentos1386/nestjs-observability/tree/e84c0b92be9416c0d29255007f3a107f6d0d900c

Steps to reproduce:

1. npm ci
2. npx nx serve example
3. curl http://localhost:3000/api
4. Observe that there is no "YOU SHOULD SEE ME" in console.
5. Edit example/src/main.ts and comment out line 17.
6. curl http://localhost:3000
7. Observe that there is now "YOU SHOULD SEE ME" in console.

[kamilmysliwiec]

Can you provide a minimum(!!) reproduction repository? (see instructions under this link but long story short, no extra libs, irrelevant dependencies, Nx, etc)

[crice88]

Not sure if this will help any. I did notice a change where I had excluded routes that contained the globalPrefix i/e api/cats. In v9.0.0 this worked fine, after upgrading to v9.4.2 my routes were no longer being filtered. Removing the prefix from the excluded routes solved my issue i/e cats.

[ealmansi]

I can confirm the bug is happening in both Nest 9.x and 10.x

Here's a minimum reproduction repo: https://stackblitz.com/edit/nestjs-11572?file=src%2Fmain.ts

Steps to reproduce:

1. Open the link above and wait until app is fully up and running.
2. Verify that the message "Running MyMiddleware.use" is displayed every time you visit the app.
3. Uncomment line 57 and restart the app.
4. Edit the app URL to ensure that you're visiting the route "/api".
5. Verify that the message is no longer displayed when you visit the app.

[ealmansi]

After some investigation, I believe that the issue comes from how the middleware path is being generated from the route info object. Take a look at this test:

// packages/core/test/middleware/route-info-path-extractor.spec.ts
      Reflect.set(routeInfoPathExtractor, 'prefixPath', '/api');

      expect(
        routeInfoPathExtractor.extractPathsFrom({
          path: '*',
          method: RequestMethod.ALL,
        }),
      ).to.eql(['/api/*']);

The test is saying: if the global prefix is /api, and the path is *, then the middleware route should be /api/*.

However, this pattern does not match the root route /api - which in turn is causing this bug. If you actually visit the route /api/, the middleware gets executed again.

It would be simple enough to update extractPathsFrom such that it no longer adds that slash in the middle, but given that there's a quite a few tests for the RouteInfoPathExtractor depending on this behaviour, I'm not sure whether this is a bug or a feature.

[CodyTseng]

This may be a bug I wrote, I will try to fix it ASAP

[B4nan]

Any reason why #12179 is not yet merged? This problem breaks the request context middleware for MikroORM users when combined with setGlobalPrefix(), I can confirm the same observations as @ealmansi. Related issue at our end here.

[itseramin]

For the path /api/v1/manager/files/:fileId, this was the correct RouteInfo object...

{
  method: RequestMethod.GET,
  path: '/manager/files/:fileId',
  version: '1',
}

Would love to contribute to the Nest documentation, if possible!

[kamilmysliwiec]

Let's track this here https://github.com/nestjs/nest/pull/13337

[B4nan]

It would be great if you could reopen and revisit this, it's clearly not fixed (and it's not just me who confirms this), at least not the part that the MikroORM integration needs. I can confirm the original PR (#12179) does help, unlike the work you ended up doing in #13337.

[kamilmysliwiec]

The original PR would cause a significant breaking change and cannot be merged as it would lead to major regressions elsewhere.

I'll reopen this issue and if someone is willing to create a PR that fixes this specific use-case (and doesn't break anything else), I'd love to merge it as soon as I can.

[B4nan]

How was it breaking? I recall there were no tests broken by it, so how can someone tell if it's breaking or not without it?

[kamilmysliwiec]

It changed the default request method from "use" to "all" which knowing how different HTTP drivers work, would evidently affect many existing projects.

[B4nan]

I don't know what that means really, I hope someone who actually uses nest and knows the internals can proceed with this, I am unfortunately not the right person. I don't mind solving code puzzles, but I would have to have some tests to show me what I cannot do - to show whether the behavior is breaking or not. It's really hard to expect any kind of "correct PR" to come from outside without that.

Or were the changes in tests what you consider breaking? As to me they look sane and expected (and they kinda confirm what the issue is about IMO).

[CodyTseng]

I mentioned in https://github.com/nestjs/nest/issues/13401#issuecomment-2066288412 that this error was caused by another PR. I haven't thought of a good solution for now.

[CodyTseng]

Perhaps adding the same filtering logic to ExpressAdapter as in FastifyAdapter could solve the issue of middleware being called multiple times.

https://github.com/nestjs/nest/blob/3272606c70e97c50492e7f1e8661a58b53921c48/packages/platform-fastify/adapters/fastify-adapter.ts#L575-L584

Then we can revert https://github.com/nestjs/nest/pull/11832 to fix this issue. I'm not sure if doing this will cause other issues.