refactor: ignore non-objects in metadata accessor

[jozsefsallai]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other... Please describe:

What is the current behavior?

Issue Number: N/A

Currently, the onModuleInit hook of the schedule module will traverse every controller and provider to determine the schedulers that are set up in them or anywhere in their dependency tree. If the prototype of any of these classes contains a method that no longer exists later on, the explorer will try to process that method too, eventually resulting in the following error which prevents the Nest application from starting:

/home/joe/..../node_modules/reflect-metadata/Reflect.js:354
                throw new TypeError();
                      ^
TypeError:
    at Reflect.getMetadata (/home/joe/..../node_modules/reflect-metadata/Reflect.js:354:23)
    at Reflector.get (/home/joe/..../node_modules/@nestjs/core/services/reflector.service.js:24:24)
    at SchedulerMetadataAccessor.getSchedulerType (/home/joe/..../node_modules/@nestjs/schedule/dist/schedule-metadata.accessor.js:21:31)
    at ScheduleExplorer.lookupSchedulers (/home/joe/..../node_modules/@nestjs/schedule/dist/schedule.explorer.js:46:48)
    at /home/joe/..../node_modules/@nestjs/schedule/dist/schedule.explorer.js:40:24
    at MetadataScanner.scanFromPrototype (/home/joe/..../node_modules/@nestjs/core/metadata-scanner.js:34:31)
    at /home/joe/..../node_modules/@nestjs/schedule/dist/schedule.explorer.js:39:34
    at Array.forEach (<anonymous>)
    at ScheduleExplorer.explore (/home/joe/..../node_modules/@nestjs/schedule/dist/schedule.explorer.js:34:26)
    at ScheduleExplorer.onModuleInit (/home/joe/..../node_modules/@nestjs/schedule/dist/schedule.explorer.js:27:14)

Now, this will likely not be an issue for the majority of Nest apps, however, in our particular case, we have a PrismaService which extends from the PrismaClient and we had to do some dependency injection magic that sets up some logging mechanisms and then registers a few extensions. According to the Prisma docs, extending a PrismaClient will get rid of some of the methods (which, by extension, were also methods in our PrismaService), so the reflector will fail when it reaches them, throwing the abovementioned exception.

The relevant line in reflect-metadata: https://github.com/rbuckton/reflect-metadata/blob/3aeb98af4030be664a66f49bfd164936e0ba1825/Reflect.ts#L994

What is the new behavior?

The behavior hasn't changed much, as the methods from the metadata accessor class can already return undefined if they wanted to. What I did was making sure that the instance we pass to these methods is an object before attempting to do reflection on them (using the same logic that's found inside the reflect-metadata package), so that they won't trigger the exception.

For now, we're using patch-package in our codebase and it does work as intended. Tests also pass.

Does this PR introduce a breaking change?

• Yes
• No

Other information

Perhaps a similar fix should be implemented for all official Nest modules that use this method. Currently the only module we use that had this problem was the scheduler module.

[kamilmysliwiec]

Suggested change

	if (isObject) {
	return this.reflector.get(key, target);
	}
	return isObject ? this.reflector.get(key, target) : undefined;

[kamilmysliwiec]

Could you move this method to the bottom of the class?

[jozsefsallai]

Done, thank you for the review.

[kamilmysliwiec]

lgtm