Skip to content
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

Nest crash when RouteConstraints is used without app versioning enabled #13496

Closed
3 of 15 tasks
johaven opened this issue Apr 26, 2024 · 4 comments
Closed
3 of 15 tasks

Comments

@johaven
Copy link
Contributor

johaven commented Apr 26, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

After some testing, there is an untested case that crashes Nest when using RouteConstraints and app versioning is not set in fastify context.

Related to: #12567

~/node_modules/@nestjs/platform-fastify/adapters/fastify-adapter.js:62
                if (this.versioningOptions.type === common_1.VersioningType.MEDIA_TYPE) {
                                           ^
TypeError: Cannot read properties of undefined (reading 'type')
    at Object.deriveConstraint (~/node_modules/@nestjs/platform-fastify/adapters/fastify-adapter.js:62:44)
    at Constrainer.eval (eval at _buildDeriveConstraints (~/node_modules/find-my-way/lib/constrainer.js:168:34), <anonymous>:4:36)
    at Constrainer.deriveConstraints (~/node_modules/find-my-way/lib/constrainer.js:62:30)
    at Router.lookup (~/node_modules/find-my-way/index.js:526:42)
    at Server.preRouting (~/node_modules/fastify/fastify.js:905:14)
    at Server.emit (node:events:518:28)
    at parserOnIncoming (node:_http_server:1137:12)
    at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17)

Minimum reproduction code

Steps to reproduce are easy, no need repo

Steps to reproduce

  1. Add @RouteConstraints({ version: '1.2.x' }) to a route
  2. Do not enable app versioning
  3. Accessing to any route on app

Expected behavior

A catch with a warning to prevent user to enable app versioning before using RouteConstraints decorator.
@Fcmam5 this maybe requires to add an additional test to catch this case.

Package

  • I don't know. Or some 3rd-party package
  • @nestjs/common
  • @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

10.3.6

Packages versions

"@nestjs/common": "10.3.6",
"@nestjs/core": "10.3.6",
"@nestjs/platform-fastify": "10.3.6",

Node.js version

20.12.1

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@johaven johaven added the needs triage This issue has not been looked into label Apr 26, 2024
@Fcmam5
Copy link
Contributor

Fcmam5 commented Apr 26, 2024

@micalevisk please assign me this issue, I'll file a PR to add a warning if app versioning is not enabled

@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue?

@micalevisk micalevisk removed the needs triage This issue has not been looked into label May 3, 2024
@Fcmam5
Copy link
Contributor

Fcmam5 commented May 4, 2024

Would you like to create a PR for this issue?

Yes, will do

Fcmam5 added a commit to Fcmam5/fastify-route-constraints-poc that referenced this issue May 4, 2024
Fcmam5 added a commit to Fcmam5/fastify-route-constraints-poc that referenced this issue May 5, 2024
@kamilmysliwiec
Copy link
Member

Let's track this here #13536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants