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

pacyL2K19 commented 7 months ago

Is there an existing issue for this?

[X] 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
```ts
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
When missing the version, it recognizes I am trying to hit the v1/ but sends a 404 error while the v1/ handler exist
When using a bad version, it recognizes I am trying to hit the v2/ but still send a 404 while v2/ handler also exists

Minimum reproduction code

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

Steps to reproduce

yarn install
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
[X] code>@nestjs/common</code
[X] code>@nestjs/core</code
[ ] code>@nestjs/microservices</code
[ ] code>@nestjs/platform-express</code
[ ] code>@nestjs/platform-fastify</code
[ ] code>@nestjs/platform-socket.io</code
[ ] code>@nestjs/platform-ws</code
[ ] code>@nestjs/testing</code
[ ] code>@nestjs/websockets</code
[X] 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?

[X] macOS
[ ] Windows
[ ] Linux

Other

No response

ari-github commented 6 months ago

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

pacyL2K19 commented 6 months ago

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

nestjs / nest