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

Route Handler Fails to Reflect Middleware Modifications to request.originalUrl for API Versioning #13211

Open
5 of 15 tasks
pacyL2K19 opened this issue Feb 14, 2024 · 2 comments
Open
5 of 15 tasks
Labels
needs triage This issue has not been looked into

Comments

@pacyL2K19
Copy link

pacyL2K19 commented Feb 14, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

I wanted to add versioning support to an existing Nest.js REST API.
Previously, all the routes were handled without version in the URL (like a GET at /users) and now I want:

  • All the requests without version in the path to be handled by v1 routes, means if I perform a GET at /users/ this needs to be handled by /v1/users handler
  • All the requests with valid versions in the URL, to be handled by the right controller, I set v1 to be the default version and added controllers for the v2
  • All the requests with an invalid version, or a version which is not supported yet, like v4 or v3, ... to be handled by the v2 controllers

My solution was to:

  • First add the version support to the app doing something like :
import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';
import { VersioningType } from '@nestjs/common';

async function bootstrap() {
  const app = await NestFactory.create(AppModule);
  app.enableVersioning({
    type: VersioningType.URI,
    defaultVersion: '1', // all routes without version prefix will be treated as v1 default prefix being `v`
  });
  await app.listen(3000);
}
bootstrap();
  • Create additional controllers to handle v2 requests, here is an example:
  @Version('2')
  @Get()
  getHelloV2(): string {
    return this.appService.getHelloV2();
  }
  • Then create a middleware that will handle the version detection logic, and map the request to the right handler, here is the middleware
import { Injectable, NestMiddleware } from '@nestjs/common';
import { NextFunction, Request, Response } from 'express';

@Injectable()
export class VersionManagementMiddleware implements NestMiddleware {
  use(req: Request, res: Response, next: NextFunction) {
    const VALID_VERSIONS = ['v1', 'v2']; // can be fetched from a config file

    // Extract the first segment of the path
    const firstPathSegment = req.originalUrl
      .split('/')[1]
      ?.toString()
      ?.toLowerCase();

    const versionPattern = /^v\d+/;

    // Check if the first segment is a version
    if (!versionPattern.test(firstPathSegment)) {
      // If not, prepend 'v1' to the path
      req.originalUrl = '/v1' + req.originalUrl;
    } else if (!VALID_VERSIONS.includes(firstPathSegment)) {
      // If an invalid version is detected e. v5 or v6, set to latest version ('v2' in this case)
      req.originalUrl = req.originalUrl.replace(firstPathSegment, 'v2');
      // notify the client that the version sent in the request is invalid
      res.locals.invalidVersion = true;
    }

    next();
  }
}

The purpose of the VersionManagementMiddleware is to ensure that all incoming requests are automatically directed to the appropriate version of our API based on the URL path. Specifically, it aims to:

    • Redirect requests lacking a version specification to our default v1 endpoints, effectively treating unversioned requests as requests for the first version of the API.
    • Serve requests with a recognized version (e.g., /v1, /v2) by the corresponding versioned route handlers, ensuring that clients accessing different API versions are correctly supported.
    • Default requests with unrecognized or unsupported versions (e.g., /v3, /vx) to the v2 endpoints, as v2 represents the latest version of our API. This behavior is intended to gracefully handle future versioning and avoid breaking changes for clients using version numbers beyond the current scope.

It's consumed in the app as following:

// app.module.ts
@Module({
  imports: [],
  controllers: [AppController],
  providers: [AppService],
})
export class AppModule {
  configure(consumer: MiddlewareConsumer) {
    consumer.apply(VersionManagementMiddleware).forRoutes('*'); // apply for all routes
  }
}

With this in place, i expected all the requests missing the version to be handled as v1, all with wrong version to be handled as v2 and everything else as expected, means a GET at v1/ or /v2 should be ignored by the middleware, but this is what currently happens:

  • When versions exist in the request -> everything works as expected -- see the screenshot below
    Screenshot 2024-02-14 at 14 34 21
    Screenshot 2024-02-14 at 15 17 53

  • When missing the version, it recognizes I am trying to hit the v1/ but sends a 404 error while the v1/ handler exist
    Screenshot 2024-02-14 at 14 34 41

  • When using a bad version, it recognizes I am trying to hit the v2/ but still send a 404 while v2/ handler also exists
    Screenshot 2024-02-14 at 14 34 58

Minimum reproduction code

https://github.com/pacyL2K19/request-original-url-mutation-middleware

Steps to reproduce

  1. yarn install
  2. yarn start:dev

Expected behavior

I expected the middleware to mutate the request.originalUrl so that requests without version are handled by v1 and those with bad version to be handled by v2 that way, my clients won't have to change anything on their side, this means the app can still work with endpoints missing version(or with unsupported versions) and the ones with version

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

express

NestJS version

10.3.2

Packages versions

{
  "name": "request-original-url-mutation-middleware",
  "version": "0.0.1",
  "description": "",
  "author": "",
  "private": true,
  "license": "UNLICENSED",
  "scripts": {
    "build": "nest build",
    "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
    "start": "nest start",
    "start:dev": "nest start --watch",
    "start:debug": "nest start --debug --watch",
    "start:prod": "node dist/main",
    "lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix",
    "test": "jest",
    "test:watch": "jest --watch",
    "test:cov": "jest --coverage",
    "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
    "test:e2e": "jest --config ./test/jest-e2e.json"
  },
  "dependencies": {
    "@nestjs/common": "^10.0.0",
    "@nestjs/core": "^10.0.0",
    "@nestjs/platform-express": "^10.0.0",
    "reflect-metadata": "^0.2.0",
    "rxjs": "^7.8.1"
  },
  "devDependencies": {
    "@nestjs/cli": "^10.0.0",
    "@nestjs/schematics": "^10.0.0",
    "@nestjs/testing": "^10.0.0",
    "@types/express": "^4.17.17",
    "@types/jest": "^29.5.2",
    "@types/node": "^20.3.1",
    "@types/supertest": "^6.0.0",
    "@typescript-eslint/eslint-plugin": "^6.0.0",
    "@typescript-eslint/parser": "^6.0.0",
    "eslint": "^8.42.0",
    "eslint-config-prettier": "^9.0.0",
    "eslint-plugin-prettier": "^5.0.0",
    "jest": "^29.5.0",
    "prettier": "^3.0.0",
    "source-map-support": "^0.5.21",
    "supertest": "^6.3.3",
    "ts-jest": "^29.1.0",
    "ts-loader": "^9.4.3",
    "ts-node": "^10.9.1",
    "tsconfig-paths": "^4.2.0",
    "typescript": "^5.1.3"
  },
  "jest": {
    "moduleFileExtensions": [
      "js",
      "json",
      "ts"
    ],
    "rootDir": "src",
    "testRegex": ".*\\.spec\\.ts$",
    "transform": {
      "^.+\\.(t|j)s$": "ts-jest"
    },
    "collectCoverageFrom": [
      "**/*.(t|j)s"
    ],
    "coverageDirectory": "../coverage",
    "testEnvironment": "node"
  }
}

Node.js version

20.11.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@pacyL2K19 pacyL2K19 added the needs triage This issue has not been looked into label Feb 14, 2024
@pacyL2K19 pacyL2K19 changed the title Route Handler Fails to Reflect Middleware Modifications to req.originalUrl for API Versioning Route Handler Fails to Reflect Middleware Modifications to request.originalUrl for API Versioning Feb 14, 2024
@ari-github
Copy link

You need to change the req.url
https://stackoverflow.com/questions/70822989/how-to-rewrite-url-path-in-nestjs

@pacyL2K19
Copy link
Author

You need to change the req.url https://stackoverflow.com/questions/70822989/how-to-rewrite-url-path-in-nestjs

Thank you for the workaround
This works as well for me 🎉

However, we still have an issue here in the response

Since the API has handlers for both GET v1/ and GET v2/, these responses are misleading

{
    "message": "Cannot GET /v2",
    "error": "Not Found",
    "statusCode": 404
}
{
    "message": "Cannot GET /v1/",
    "error": "Not Found",
    "statusCode": 404
}

I think it should say something different, not that it can't handle them @micalevisk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

2 participants