fix(uploads): add return on null-stream 404 + guard sharp transform errors

Address two CodeRabbit findings:

1. Add return before responses.error(res, 404) in both get() and getSharp()
   so execution does not fall through to stream.on() when getStream returns
   falsy — preventing a TypeError and ERR_HTTP_HEADERS_SENT crash on the
   null-stream path.

2. Attach the headersSent-gated error handler to the sharp Transform (not
   just the GridFS source stream). pipe() does not forward error events
   between streams, so sharp-pipeline failures were unhandled and could
   crash the worker process. Refactored to buildTransform() helper that
   creates the transform, attaches the error handler, and returns it.
   Update comment to accurately describe when headersSent flips to true
   (first chunk written, not res.set()).

Add test: sharp transform error after headers sent → res.destroy + logger.
Full unit suite: 1623/1623 pass.

Author: PierreBrisorgueil

modules/uploads/controllers/uploads.controller.js

@@ -39,11 +39,11 @@ const normalizeMime = (raw) => String(raw).toLowerCase().split(';')[0].trim();
 const get = async (req, res) => {
   try {
     const stream = await UploadsService.getStream({ _id: req.upload._id });
-    if (!stream) responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
+    if (!stream) return responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
     stream.on('error', (err) => {
-      // Guard against ERR_HTTP_HEADERS_SENT when GridFS fails mid-transfer:
-      // `res.set('Content-Type', ...)` below flushes response headers to the
-      // client before the stream starts. If the GridFS read then errors, a
+      // Guard against ERR_HTTP_HEADERS_SENT when GridFS fails mid-transfer.
+      // Response headers are written when the first chunk is piped to res
+      // (`stream.pipe(res)` below). If GridFS errors after that point, a
       // second status+body write would throw ERR_HTTP_HEADERS_SENT and crash
       // the worker process. Pre-header errors still reach the client as 422;
       // post-header errors destroy the socket so Express surfaces them via
@@ -76,37 +76,45 @@ const get = async (req, res) => {
 const getSharp = async (req, res) => {
   try {
     const stream = await UploadsService.getStream({ _id: req.upload._id });
-    if (!stream) responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
-    stream.on('error', (err) => {
-      // Same headersSent guard as `get` above. A sharp pipeline error after
-      // `res.set('Content-Type', ...)` has flushed the response head cannot
-      // write a new status or body — attempting to do so throws
-      // ERR_HTTP_HEADERS_SENT. Destroy the socket instead so Express surfaces
-      // the error via its default handler without double-sending.
+    if (!stream) return responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
+    // headersSent guard for the GridFS source stream.
+    // pipe() does not forward 'error' events between streams; each stream in the
+    // chain needs its own listener. Source and Transform errors are handled below.
+    // Pre-header errors reach the client as 422; post-header errors destroy the
+    // socket so Express surfaces them via its default handler instead of
+    // attempting a second write that would throw ERR_HTTP_HEADERS_SENT.
+    const onStreamError = (label) => (err) => {
       if (!res.headersSent) {
         responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
       } else {
-        logger.error('uploads.getSharp - stream error after headers sent', err);
+        logger.error(`uploads.getSharp - ${label} error after headers sent`, err);
         res.destroy(err);
       }
-    });
+    };
+    stream.on('error', onStreamError('source stream'));
     const raw = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
     const norm = normalizeMime(raw);
     const contentType = SAFE_IMAGE_MIME.has(norm) ? norm : 'image/jpeg';
     res.set('Content-Type', contentType);
-    switch (req.sharpOption) {
-      case 'blur':
-        stream.pipe(sharp().resize(req.sharpSize).blur(config.uploads.sharp.blur)).pipe(res);
-        break;
-      case 'bw':
-        stream.pipe(sharp().resize(req.sharpSize).grayscale()).pipe(res);
-        break;
-      case 'blur&bw':
-        stream.pipe(sharp().resize(req.sharpSize).grayscale().blur(config.uploads.sharp.blur)).pipe(res);
-        break;
-      default:
-        stream.pipe(sharp().resize(req.sharpSize)).pipe(res);
-    }
+    const buildTransform = () => {
+      let transform;
+      switch (req.sharpOption) {
+        case 'blur':
+          transform = sharp().resize(req.sharpSize).blur(config.uploads.sharp.blur);
+          break;
+        case 'bw':
+          transform = sharp().resize(req.sharpSize).grayscale();
+          break;
+        case 'blur&bw':
+          transform = sharp().resize(req.sharpSize).grayscale().blur(config.uploads.sharp.blur);
+          break;
+        default:
+          transform = sharp().resize(req.sharpSize);
+      }
+      transform.on('error', onStreamError('sharp transform'));
+      return transform;
+    };
+    stream.pipe(buildTransform()).pipe(res);
   } catch (err) {
     responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
   }

modules/uploads/tests/uploads.controller.contenttype.unit.tests.js

@@ -36,6 +36,9 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
   };
 
   let mockLogger;
+  // Captures the 'error' listener registered on the sharp Transform so tests
+  // can fire it manually to simulate a sharp-pipeline mid-stream failure.
+  let sharpListeners;
 
   beforeEach(async () => {
     jest.resetModules();
@@ -47,6 +50,7 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
     };
 
     mockLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn() };
+    sharpListeners = {};
 
     jest.unstable_mockModule('../services/uploads.service.js', () => ({
       default: mockUploadsService,
@@ -60,12 +64,16 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
     // sharp is a real dependency; mock it to avoid needing native bindings in unit context.
     // pipe must return `dest` (matching the real Stream.pipe contract) so that chained
     // calls stream.pipe(transform).pipe(res) work without throwing.
+    // on() captures listeners so tests can fire transform error events.
     jest.unstable_mockModule('sharp', () => ({
       default: jest.fn(() => ({
         resize: jest.fn().mockReturnThis(),
         blur: jest.fn().mockReturnThis(),
         grayscale: jest.fn().mockReturnThis(),
         pipe: jest.fn((dest) => dest),
+        on: jest.fn((event, handler) => {
+          sharpListeners[event] = handler;
+        }),
       })),
     }));
 
@@ -492,20 +500,37 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
       expect(res.destroy).not.toHaveBeenCalled();
     });
 
-    test('calls res.destroy(err) and logger.error when stream errors after headers are sent', async () => {
+    test('calls res.destroy(err) and logger.error when source stream errors after headers are sent', async () => {
       const { listeners } = installStream();
       const { default: responses } = await import('../../../lib/helpers/responses.js');
       const res = buildRes();
 
       await UploadsController.getSharp(req, res);
 
       res.headersSent = true;
-      const err = new Error('sharp pipeline mid-stream abort');
+      const err = new Error('gridfs source mid-stream abort');
       listeners.error(err);
 
       expect(responses.error).not.toHaveBeenCalled();
       expect(res.destroy).toHaveBeenCalledWith(err);
       expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('uploads.getSharp'), err);
     });
+
+    test('calls res.destroy(err) and logger.error when sharp transform errors after headers are sent', async () => {
+      installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.getSharp(req, res);
+
+      // Simulate a sharp transform error mid-stream (after headers are flushed).
+      res.headersSent = true;
+      const err = new Error('sharp transform mid-stream abort');
+      sharpListeners.error(err);
+
+      expect(responses.error).not.toHaveBeenCalled();
+      expect(res.destroy).toHaveBeenCalledWith(err);
+      expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('uploads.getSharp'), err);
+    });
   });
 });