fix(express): leave 5xx http-errors errors unmapped so they are logged (#18232)

Since #18214, every http-errors error that reaches the Express error handler is mapped to an HttpException, which the built-in exception filter does not log. Server faults such as EACCES from serving a static file, or a 5xx a middleware passes to next(), were answered without a log line. Only errors below 500 are mapped now, so a 5xx one is logged again, as on Fastify.

Co-authored-by: rk <rk@garuna.dev>

integration/hello-world/e2e/exceptions.spec.ts

@@ -3,6 +3,8 @@ import {
   Get,
   HttpStatus,
   INestApplication,
+  InternalServerErrorException,
+  Logger,
   Post,
   UnauthorizedException,
 } from '@nestjs/common';
@@ -13,6 +15,7 @@ import {
 } from '@nestjs/platform-fastify';
 import { Test } from '@nestjs/testing';
 import { RawServerDefault } from 'fastify';
+import createError from 'http-errors';
 import request from 'supertest';
 import { ErrorsController } from '../src/errors/errors.controller.js';
 import { listenOnLoopback } from '../../_support/listen-on-loopback.js';
@@ -25,6 +28,14 @@ class ErrorBodyController {
   }
 }
 
+@Controller()
+class ErrorCauseController {
+  @Get('error-cause')
+  findAll() {
+    throw new InternalServerErrorException('Oops', { cause: createError(404) });
+  }
+}
+
 @Controller()
 class SchemaController {
   @RouteSchema({ body: { type: 'object', required: ['name'] } })
@@ -39,10 +50,17 @@ describe('Error messages', () => {
     let app: INestApplication;
     beforeEach(async () => {
       const module = await Test.createTestingModule({
-        controllers: [ErrorsController, ErrorBodyController],
+        controllers: [
+          ErrorsController,
+          ErrorBodyController,
+          ErrorCauseController,
+        ],
       }).compile();
 
       app = module.createNestApplication();
+      app.use('/middleware-error', (req, res, next) =>
+        next(createError(503, 'connect ECONNREFUSED 127.0.0.1:6379')),
+      );
       server = app.getHttpServer();
       await listenOnLoopback(app);
     });
@@ -99,8 +117,45 @@ describe('Error messages', () => {
         });
     });
 
+    it(`/GET (ServiceUnavailable logged when a middleware passes an "http-errors" error to next())`, async () => {
+      const logError = vi
+        .spyOn(Logger.prototype, 'error')
+        .mockImplementation(() => {});
+
+      await request(server)
+        .get('/middleware-error')
+        .expect(HttpStatus.SERVICE_UNAVAILABLE)
+        .expect({
+          statusCode: 503,
+          message: 'Service Unavailable',
+        });
+      expect(logError).toHaveBeenCalledExactlyOnceWith(
+        expect.objectContaining({
+          statusCode: 503,
+          message: 'connect ECONNREFUSED 127.0.0.1:6379',
+        }),
+      );
+    });
+
+    it(`/GET (InternalServerError not logged when its cause is a 4xx "http-errors" error)`, async () => {
+      const logError = vi
+        .spyOn(Logger.prototype, 'error')
+        .mockImplementation(() => {});
+
+      await request(server)
+        .get('/error-cause')
+        .expect(HttpStatus.INTERNAL_SERVER_ERROR)
+        .expect({
+          statusCode: 500,
+          error: 'Internal Server Error',
+          message: 'Oops',
+        });
+      expect(logError).not.toHaveBeenCalled();
+    });
+
     afterEach(async () => {
       await app.close();
+      vi.restoreAllMocks();
     });
   });
 

packages/platform-express/adapters/express-adapter.ts

@@ -690,7 +690,9 @@ export class ExpressAdapter extends AbstractHttpAdapter<
       // and an expose flag. Without this case, errors like PayloadTooLargeError reach custom
       // exception filters as a raw http-errors instance rather than an HttpException, so a
       // @Catch(HttpException) filter never runs and a catch-all filter reports 500 (#18213).
-      case isHttpError(error):
+      // A 5xx one (e.g. EACCES from "send") is a server fault: it stays unmapped,
+      // so the built-in filter still logs it, as it does on Fastify.
+      case isHttpError(error) && error.statusCode < 500:
         return new HttpException(
           // An error that must not reach the client only gets a status text,
           // like the built-in exception filter answers it

packages/platform-express/test/adapters/express-adapter.spec.ts

@@ -540,16 +540,18 @@ describe('ExpressAdapter', () => {
       expect(result.cause).toBe(error);
     });
 
-    it.each([
-      [500, 'Internal Server Error'],
-      [599, 'Internal server error'],
-    ])(
-      'should not expose the message of an http-errors error with status %i',
-      (status, message) => {
+    it('should not expose the message of an http-errors error with expose: false', () => {
+      const error = createHttpError(404, 'internal detail', { expose: false });
+      const result = expressAdapter.mapException(error) as HttpException;
+      expect(result.getStatus()).toBe(404);
+      expect(result.getResponse()).toBe('Not Found');
+    });
+
+    it.each([500, 503])(
+      'should not map an http-errors error with status %i',
+      status => {
         const error = createHttpError(status, 'internal detail');
-        const result = expressAdapter.mapException(error) as HttpException;
-        expect(result.getStatus()).toBe(status);
-        expect(result.getResponse()).toBe(message);
+        expect(expressAdapter.mapException(error)).toBe(error);
       },
     );
   });