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

Multi-tenant app fail on first non durable tree request #13477

Closed
3 of 15 tasks
DylanVeldra opened this issue Apr 22, 2024 · 2 comments
Closed
3 of 15 tasks

Multi-tenant app fail on first non durable tree request #13477

DylanVeldra opened this issue Apr 22, 2024 · 2 comments

Comments

@DylanVeldra
Copy link

DylanVeldra commented Apr 22, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

For the context, the app is a multi-tenants, with a configDb service being instantiate for each tenant when needed.

The issue manifest when the first request on a tenant isn't on a isTreeDurable === true

I found how to patch it and I also found a work around.

The patch:
packages/core/router/router-explorer.ts line 421 replace

const requestProviderValue = isTreeDurable ? contextId.payload : request;

by

const requestProviderValue = isTreeDurable ? contextId.payload : Object.assign(request, contextId.payload);

same for packages/core/middleware/middleware-module.ts line 349

The workaround is commented in the reproduction repo.
I would like to know if the use of the work around is safe or the patch should be applied ?

Minimum reproduction code

https://github.com/DylanVeldra/nest-multitenant

Steps to reproduce

This will work:
npm i
npm start

curl --location 'localhost:8080/foo' \
--header 'x-tenant-id: BAR'
curl --location 'localhost:8080/kaz' \
--header 'x-tenant-id: BAR'

This wont work if you change the order of curl request like this:

npm i
npm start

curl --location 'localhost:8080/kaz' \
--header 'x-tenant-id: BAR'
curl --location 'localhost:8080/foo' \
--header 'x-tenant-id: BAR'

Expected behavior

Both orders should work, no matter if the first request is a isTreeDurable or not

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.8

Packages versions

{
  "name": "nest-multitenant",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "build": "nest build",
    "dev": "npm run build && nest start --watch",
    "start": "npm run build && nest start --watch"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "@nestjs/common": "^10.3.8",
    "@nestjs/core": "^10.3.8",
    "@nestjs/platform-express": "^10.3.8",
    "express": "^4.19.2"
  },
  "devDependencies": {
    "@types/express": "^4.17.21"
  }
}

Node.js version

21.4.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

Please let me know if you need more informations,

Have good day 👋

@DylanVeldra DylanVeldra added the needs triage This issue has not been looked into label Apr 22, 2024
@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue? Patch you suggested sounds reasonable

@kamilmysliwiec
Copy link
Member

Let's track this here #13485

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

3 participants