perf: Proposal for Improved Error Handling while token might be undefined

[mostafa8026]

Is there an existing issue that is already proposing this?

• I have searched the existing issues

NestJS version

10.2.7

Is your performance suggestion related to a problem? Please describe it

We encountered a problem in our barrel file where a constant was undefined. After some effort, we identified and resolved the issue. However, I believe enhancing the injector to raise an error would significantly aid developers in understanding such issues more efficiently.

Describe the performance enhancement you are proposing and how we can try it out

The enhancement I propose is not directly code-related but pertains to debugging efficiency. If an error were thrown when a token is undefined, it would expedite problem-solving. Here is a suggested implementation:
reference file

export function Inject<T = any>(
  token?: T,
): PropertyDecorator & ParameterDecorator {
  return (target: object, key: string | symbol | undefined, index?: number) => {
    const type = token || Reflect.getMetadata('design:type', target, key);
    if(!type) throw new Error('You should provide the token properly, undefined is not acceptable');
    // rest of the code.
}

Benchmarks result or another proof (eg: POC)

As previously mentioned, this enhancement can significantly reduce debugging time. While I do not have specific benchmarks, the time saved during the debugging process serves as practical proof of its effectiveness.

[micalevisk]

I don't think that stating 'undefined' would help

those errors are likely due to circular imports (as clarified here). I believe that a better approach would be rewording the error message or just improve the docs (the FAQ page)

[mostafa8026]

Thank you for your insightful response. Your explanation about potential circular imports is enlightening. I agree that rewording the error message or enhancing the FAQ documentation could be beneficial. To further this idea, I suggest to use this descriptive error:

if(!type) throw new Error(`Token is undefined. This often occurs due to circular dependencies.
Ensure there are no circular dependencies in your files or barrel files. 
For more details, refer to https://trilon.io/blog/avoiding-circular-dependencies-in-nestjs.`);

This could potentially make the error more clear for developer.

[kamilmysliwiec]

Would you like to create a PR for this issue?

[mostafa8026]

Kamil, I'm truly honored by your response and the opportunity to contribute to Nest.js. Your work and dedication to this framework have been a great inspiration to me. YES, I am excited to take on this task! I'll approach this pull request with great passion and care, aiming to make a valuable contribution to the community. Looking forward to working on this!

[Clashsoft]

This broke some libraries.

event.service.ts:

@Injectable()
export class EventService {
  constructor(
    @Inject('EVENT_SERVICE') private client: ClientProxy,
    private eventEmitter2: EventEmitter2,
    @Optional() @Inject() private logger: Logger = new Logger(EventService.name),
  ) {
  }

event.service.js:

let EventService = EventService_1 = class EventService {
    constructor(client, eventEmitter2, logger = new common_1.Logger(EventService_1.name)) {
        this.client = client;
        this.eventEmitter2 = eventEmitter2;
        this.logger = logger;
    }
// ....
exports.EventService = EventService = EventService_1 = tslib_1.__decorate([
    (0, common_1.Injectable)(),
    tslib_1.__param(0, (0, common_1.Inject)('EVENT_SERVICE')),
    tslib_1.__param(2, (0, common_1.Optional)()),
    tslib_1.__param(2, (0, common_1.Inject)()),
    tslib_1.__metadata("design:paramtypes", [microservices_1.ClientProxy,
        event_emitter_1.EventEmitter2,
        common_1.Logger])
], EventService);

Stacktrace:

Error: Token is undefined at index: 2. This often occurs due to circular dependencies.
Ensure there are no circular dependencies in your files or barrel files. 
For more details, refer to https://trilon.io/blog/avoiding-circular-dependencies-in-nestjs.
    at /.../node_modules/.pnpm/@nestjs+common@10.3.3_class-transformer@0.5.1_class-validator@0.14.1_reflect-metadata@0.1.14_rxjs@7.8.1/node_modules/@nestjs/common/decorators/core/inject.decorator.js:39:19
    at /.../node_modules/.pnpm/tslib@2.6.2/node_modules/tslib/tslib.js:111:41
    at DecorateConstructor (/.../node_modules/.pnpm/reflect-metadata@0.1.14/node_modules/reflect-metadata/Reflect.js:541:33)
    at Reflect.decorate (/.../node_modules/.pnpm/reflect-metadata@0.1.14/node_modules/reflect-metadata/Reflect.js:130:24)
    at Object.__decorate (/.../node_modules/.pnpm/tslib@2.6.2/node_modules/tslib/tslib.js:105:96)
    at Object.<anonymous> (/.../node_modules/.pnpm/@mean-stream+nestx@0.11.3_@nestjs+common@10.3.3_@nestjs+core@10.3.3_@nestjs+event-emitter@2.0_ui6q4tps32tzc2lkz4tf6kpu3u/libs/nestx/src/lib/event/event.service.ts:8:26)

From the stack trace the error appears at the tslib_1.__param(2, (0, common_1.Inject)()), call.
Later the Inject decorator calls Reflect.getMetadata('design:type', target, key) which returns undefined because the parameter type is stored in design:paramtypes instead of design:type...

[glebbash]

This also breaks nestjs-fireorm: glebbash/nestjs-fireorm#11.

[kamilmysliwiec]

Reverted #13370

[kamilmysliwiec]

Published as v10.3.7