Bug: NestJS Controllers not Routing [1.6.4]

[DeanAyalon]

Issue Description

• When decorating a Controller's method, the route only gets mapped if the logger-decorator runs before the @get/post/etc. decorator
• When decorating a NestJS Controller class, the route does not get mapped.
  Probably a result of the first issue, the class decorator decorating the functions, already having been decorated by their own @httpMethod decorators

To Reproduce
Steps to reproduce the behavior:

1. Create a NestJS server
2. Create a module, service and controller
3. Decorate the controller

Example

@Controller('user')              // Route '/user' does not get mapped
@log()
export class UserController {...} 

@log()
@Controller('client')            // Route '/client' does not get mapped
export class ClientController {...} 

@Controller('groups')
export class GroupsController {
    @Get()                       // Route '/groups/' mapped successfully
    @log()
    async get() {...}

    @log()
    @get('client')               // Route '/groups/client' does not get mapped
    async getClientGroups() {...}
}

Environment:

• Version 1.6.4
• Node.js version: 20.11.0
• Operating System: macOS Catalina 10.15.7 (19H2026)

[pustovitDmytro]

Unfortunately, I didn't work with NestJS. To help me better understand and reproduce the issue, could you please provide a minimum reproduce repository along with launch instructions? This will enable me to quickly replicate the bug on my end.

[DeanAyalon]

Oh no, I did not mean to spam this, sorry

[pustovitDmytro]

Hi again, think found a solution
nestJS depends on Reflect Metadata https://blog.bitsrc.io/typescripts-reflect-metadata-what-it-is-and-how-to-use-it-fb7b19cfc7e2#cf5a
I can extend logger-decorator API to also support it. but unfortunately, I didn't find a way how to see all metadata, that is defined for a class method (if you know some, you are welcome to share).

So, the API could be smth like that, and should explicitly include all metadata props, defined by other decorators:

@log({
    keepReflectMetadata: [ 'path' ] // path is the key I found useful for nestJS
})

Note that you can build your own singleton and reuse it:

import { Decorator } from 'logger-decorator';

const log = new Decorator({ keepReflectMetadata: [ ] });

if you are looking for any other possible metadata props in NestJS you can watch next files:
https://github.com/nestjs/nest/blob/master/packages/common/constants.ts
https://github.com/nestjs/nest/blob/master/packages/common/decorators/core/controller.decorator.ts

after adding 'path' metadata, looks like it helped, both /unmapped and /mapped/fails are logged and returns response.

[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [NestFactory] Starting Nest application...
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [InstanceLoader] UnroutedModule dependencies initialized +8ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [InstanceLoader] MappedModule dependencies initialized +0ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [InstanceLoader] AppModule dependencies initialized +0ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RoutesResolver] AppController {/}: +9ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RouterExplorer] Mapped {/, GET} route +2ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RoutesResolver] UnmappedController {/unmapped}: +0ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RouterExplorer] Mapped {/unmapped, undefined} route +0ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RoutesResolver] MappedController {/mapped}: +0ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RouterExplorer] Mapped {/mapped, GET} route +1ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RouterExplorer] Mapped {/mapped/works, GET} route +0ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [RouterExplorer] Mapped {/mapped/fails, undefined} route +0ms
[Nest] 16599  - 01/31/2024, 9:32:16 PM     LOG [NestApplication] Nest application successfully started +2ms
/unmapped called
{
  service: 'UnmappedController',
  method: 'get',
  level: 'info',
  params: '[]',
  result: "'/unmapped'",
  benchmark: '1.00000'
}
{
  service: 'MappedController',
  method: 'unworkingRoute',
  level: 'info',
  params: '[]',
  result: "'/mapped/fails'",
  benchmark: '0.00000'
}

however I noticed undefined in LOG [RouterExplorer] Mapped {/mapped/fails, undefined} route log, so smth can work not as expected, but I believe it could be fixed by adding additional metadata through keepReflectMetadata: [ ] flag.

I hope I will release a new version with the keepReflectMetadata feature by the end of the week

[DeanAyalon]

Unfortunately it seems Reflect Metadata does not have an option to view all class/function/method metadata, only to access specific keys - Though keepMetadata: string[] would be a great solution until such option is available

Do you think this feature could/should be requested?

As for the undefined, notice it also applies to the unmapped, meaning there's probably another metadata key tracked by RouterExplorer, maybe one that defines which HTTP Method is used, so the logger may need to keep both intact

Now just to clarify - Decorating a class with @log will apply the @log with the same config to all methods of that class, right? Meaning, keepMetadata should keep intact all the methods' metadata, not only the class itself's, right?

–––––––––––––––––––––––––––––––––––

As for the previous comments you deleted
The custom @failed decorator could prove a somewhat useful work-around the problem, if you do something like that:

// common.ts
import { Get } from '@nestjs/common'
import log from 'logger-decorator'

export Get = (path?, config?) => applyDecorators(log(config), Get(path)) 

// controller.ts
import { Get } from './common/common.ts'

That way, any @get used on this Controller will automatically be logged
It's no solution, but it's something, would certainly write less

[DeanAyalon]

After a little investigation I found it!
import { PATH_METADATA, METHOD_METADATA } from '@nestjs/common'

Those are the keys used :)

[DeanAyalon]

It seems the @<HTTPMethod> decorator itself is able to keep the metadata intact, so their code might be the key to a solution

This, for example, works, keeping both the Role, and the Post metadata:

import { SetMetadata } from "@nestjs/common";
const Role = (role: UserRole) => SetMetadata('role', role); 

@Post()
@Role(UserRole.ADMIN)
create(@Body() body: CreateDto, @GetRole() role: UserRole) {
    return this.clientService.create(role, body)
}

Edit: I might be wrong here, it both uses their internal SetMetadata function that might be storing the keys, and it does not return a proxy function, which might be the problem, though, take my words with a grain of salt, I'm kind of lost

[pustovitDmytro]

1. Do you think this feature could/should be requested?

• I don't believe accessing all metadata will become available; I guess it was designed for security purposes.

2. Decorating a class with @log will apply the @log with the same config to all methods of that class, right? Meaning, keepMetadata should keep intact all the methods' metadata, not only the class itself's, right?

• I mean apply keepMetadata only to methods, not to the class itself, because the class is not replaced by an intermediate value during decoration (as functions are).

3. As for the previous comments you deleted

• I noticed that the applyDecorators workaround depends on the order of the arguments, so this is no different from just placing decorators in the working order, as far as I can tell.

4. It seems the @ decorator itself is able to keep the metadata intact

• As I understand it, it works due to decoratorFactory.KEY = metadataKey; but I can't rely on it in the library because it is a nest-specific logic.

5. import { PATH_METADATA, METHOD_METADATA } from '@nestjs/common'

• yeah, I've also tried to add ['path', 'method' ] but it did not affect 'undefined' in the log.

[DeanAyalon]

1. Thought as much
2. What I meant is:

import { Controller, PATH_METADATA, METHOD_METADATA, Get } from '@nestjs/common'
import log from 'logger-controller'

@Controller('resource')
@log({ keepMetadata: [PATH_METADATA, METHOD_METADATA] })
export class ResourceController { ... }

So that when the log decorates the class, the keepMetadata protects all the metadata keys defined in all methods/properties within that class. As it basically decorates the methods within with the @log method decorator with the same config, is that right?

3. It's not much different, except it allows for slightly less writing, if I define a custom @get that is actually a combination of @get and @log, then I don't have to worry about getting the code wrong at any point
   It is a rather ugly workaround though, so I'm looking forward to the keepMetadata option

4. Yes, I realized after posting the comment, it was an internal Nest thing....

5. Oh no!

[DeanAyalon]

It seems I do have access to the METHOD_METADATA, printing 0 for Get, 1 for Post and 2 for Put

I went through all their other constants, they do not use other metadata

When testing it out, I found that simply setting those two metadata keys does work, so it SHOULD work - might the mistake be somewhere in your code instead?

@SetMetadata('path', 'testing') // /client/testing
@SetMetadata('method', 2)       // Put
@log()                          // Removes metadata assigned by @Post
@Post()                         // Post in path '/'
create(@Body() body: CreateDto) {
    return this.clientService.create(body)
}

Console:

[Nest] 66453  - 02/01/2024, 2:00:26 PM     LOG [RouterExplorer] Mapped {/client/testing, PUT} route +0ms

[pustovitDmytro]

yeah, metadata===0 for get, so I omitted it in if(metadata).

yeah,

import { Controller, PATH_METADATA, METHOD_METADATA, Get } from '@nestjs/common'
import log from 'logger-controller'

@Controller('resource')
@log({ keepMetadata: [PATH_METADATA, METHOD_METADATA] })
export class ResourceController { ... }

in this snippet it is an exact approach, I recommend

[pustovitDmytro]

🎉 This issue has been resolved in version 1.7.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀