In context of microservices, the client throws general "Internal server error" without any additional details

alexandrubau commented 10 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

When developing Nest applications using microservices architecture it is common to issue requests from client to server. Some of those requests may fail for whatever reason (e.g. the server throws an exception) and the client will throw a very vague exception saying Internal server error without any additional information of why it failed.

Minimum reproduction code

https://github.com/alexandrubau/nestjs-microservice-client-error

Steps to reproduce

Run nest start app-b
Run nest start app-a
Access http://localhost:3000/add from browser
View logs from app-a

Expected behavior

Instead of throwing a very vague exception saying Internal server error, the client should throw a more detailed exception. Those details may aid in debugging and figuring out which external request failed. Some of the details that may be included: message pattern, message body, exception from the server, etc.

Package

[ ] I don't know. Or some 3rd-party package
[ ] code>@nestjs/common</code
[ ] code>@nestjs/core</code
[X] 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
[ ] Other (see below)

Other package

No response

NestJS version

10.2.0

Packages versions

{
  "name": "app-a",
  "version": "0.0.1",
  "description": "",
  "author": "",
  "private": true,
  "license": "UNLICENSED",
  "scripts": {
    "build": "nest build",
    "format": "prettier --write \"apps/**/*.ts\" \"libs/**/*.ts\"",
    "start": "nest start",
    "start:dev": "nest start --watch",
    "start:debug": "nest start --debug --watch",
    "start:prod": "node dist/apps/app-a/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 ./apps/app-a/test/jest-e2e.json"
  },
  "dependencies": {
    "@nestjs/common": "^10.0.0",
    "@nestjs/core": "^10.0.0",
    "@nestjs/microservices": "^10.2.7",
    "@nestjs/platform-express": "^10.0.0",
    "reflect-metadata": "^0.1.13",
    "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": "^2.0.12",
    "@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": ".",
    "testRegex": ".*\\.spec\\.ts$",
    "transform": {
      "^.+\\.(t|j)s$": "ts-jest"
    },
    "collectCoverageFrom": [
      "**/*.(t|j)s"
    ],
    "coverageDirectory": "./coverage",
    "testEnvironment": "node",
    "roots": [
      "<rootDir>/apps/"
    ]
  }
}

Node.js version

20.7.0

In which operating systems have you tested?

[X] macOS
[ ] Windows
[ ] Linux

Other

No response

alexandrubau commented 10 months ago

Tagging some issues that might be related: #5710 and #6496.

alexandrubau commented 10 months ago

One way to get around this is to create a service that wraps the this.client.send<number>(pattern, payload); from here and throw a custom exception on which you can add the message pattern and message body. Still, there is no way to get the exception from the server.

In my opinion this behaviour should be happening by default.

kamilmysliwiec commented 10 months ago

Errors are intentionally swallowed and auto-converted to InternalServerException (IF the error thrown is not an instance of RpcException class, or an instance of a class that inherits from RpcException). In your minimum reproduction repository, you're just throwing the Error object and so it's mapped to the InternalServerException object (it works similarly for HTTP apps).

alexandrubau commented 10 months ago

Why this double standard when treating exceptions?

If I throw exception (throw new Error('MyException');) from AppA:AppController:getHello() I get a beautiful stack trace but from AppA:AppController:add() I get a generic "Internal server error" without any trace.

I do believe the behaviour should be the same in both cases.

jmcdo29 commented 10 months ago

@alexandrubau do you throw the error inside the add method inside app A or inside the method from within app B that gets called by app A? It would be a difference of where the error happens then, right?

alexandrubau commented 10 months ago

@jmcdo29 I, as a developer, should be able to identify the root cause of an exception, no matter the place from where it is being thrown. How should I be able to identify which of the microservice requests is failing?

In order to have some insights about where this might happen, every NestJS developer would have to find a workaround such as using a wrapper.

jmcdo29 commented 10 months ago

I mean, if you call an API (a weather API for example) and they return an error, do you expect your server to know why that API failed? Only if its in the returned data about the error, right? Microservices are no different, in my opinion, they are APIs that should return data about why they failed if they expect their consumers to react to why it failed.

If your microservice throws an RcpException, Nest forwards that on as expected, as it does with HttpExceptions in HTTP servers. If you just throw new Error(), the the microservice will log the stack trace and return the fact that there was an unknown exception, just like the HTTP server would, right? Once in the HTTP server, the error should be handled or converted to a format Nest recognizes (HttpException) else it gets treated like your usual Error class would

alexandrubau commented 10 months ago

Thank you for the explanations.

nestjs / nest