fix(gallery): a missing file must not take the backend down (#1128)

LocalFsStorage.get() returns an fs.createReadStream, which is lazy: it resolves
immediately and opens the file on a later tick, so an ENOENT arrives after the
await returned and outside the route's try/catch. An unhandled 'error' event is
a process-level throw Express cannot catch — the backend exits and every gallery
goes blank until the container restarts.

gallery.js had ten .pipe(res) calls and zero error handlers.

pipeStreamToResponse attaches the missing handler: a vanished source becomes a
404 (410 for a prepared zip), anything else a 500, and a source that dies
mid-response destroys the connection rather than rewriting a status already on
the wire. Headers staged for the file are cleared first — Express does not
overwrite an existing Content-Type, and a surviving Cache-Control would let a
transient 404 be cached as a broken tile for up to an hour. It also releases the
source when a client hangs up.

Applied to all eight streaming responses, not just the thumbnail route.

Stable twin of #1133, reduced: the tier-race half does not apply here because
ensureThumbnailAtWidth does not exist on this branch.

backend/__tests__/utils/streamResponse.test.js

@@ -0,0 +1,160 @@
+/**
+ * The contract that matters here is negative: a source that disappears must
+ * NOT be able to end the process (#1128).
+ *
+ * `fs.createReadStream` is lazy, so its ENOENT lands on a later tick, outside
+ * the route's try/catch. An EventEmitter emitting 'error' with no listener
+ * throws, and an uncaught throw from an I/O callback exits Node — which is how
+ * one missing thumbnail tier took every gallery on the install down.
+ *
+ * These use a REAL fs stream over a real missing path rather than a fake
+ * emitter: the point under test is the lazy-open timing, and a hand-rolled
+ * mock that emits synchronously would pass while proving nothing.
+ */
+
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { Readable } = require('stream');
+const { EventEmitter } = require('events');
+
+const { pipeStreamToResponse } = require('../../src/utils/streamResponse');
+
+jest.mock('../../src/utils/logger', () => ({
+  warn: jest.fn(), error: jest.fn(), info: jest.fn(), debug: jest.fn(),
+}));
+
+/** Minimal Express-ish response that records what happened to it. */
+function makeRes() {
+  const res = new EventEmitter();
+  res.headers = { 'Content-Length': '1234', ETag: '"x"' };
+  res.statusCode = 200;
+  res.headersSent = false;
+  res.writableEnded = false;
+  res.body = null;
+  res.destroyed = false;
+  res.removeHeader = (h) => { delete res.headers[h]; };
+  res.setHeader = (h, v) => { res.headers[h] = v; };
+  res.status = (code) => { res.statusCode = code; return res; };
+  res.json = (payload) => { res.body = payload; res.writableEnded = true; return res; };
+  res.destroy = () => { res.destroyed = true; };
+  // pipe() target surface
+  res.write = () => true;
+  res.end = () => { res.writableEnded = true; };
+  res.on = EventEmitter.prototype.on.bind(res);
+  res.emit = EventEmitter.prototype.emit.bind(res);
+  return res;
+}
+
+const settle = () => new Promise((resolve) => setTimeout(resolve, 50));
+
+describe('pipeStreamToResponse (#1128)', () => {
+  it('turns a missing file into a 404 instead of an unhandled error', async () => {
+    const missing = path.join(os.tmpdir(), `picpeak-not-here-${Date.now()}.jpg`);
+    const res = makeRes();
+
+    pipeStreamToResponse(stream_(missing), res, { context: 'thumbnail for photo 1' });
+    await settle();
+
+    expect(res.statusCode).toBe(404);
+    expect(res.body).toEqual({ error: 'File not found' });
+  });
+
+  // How this test discriminates, since the failure mode is a process-level
+  // one: replacing the call above with a bare `stream.pipe(res)` — what the
+  // thumbnail route did — makes jest fail this suite on the unhandled 'error'
+  // event before either assertion runs. Verified by doing exactly that.
+  // Catching the throw with a process.on('uncaughtException') listener does
+  // NOT work here and would be theatre: the runner installs its own handling,
+  // so such a listener never sees it and the assertion could never fail.
+  function stream_(p) { return fs.createReadStream(p); }
+
+  it('strips every header that described the file it can no longer send', async () => {
+    const res = makeRes();
+    // What the image and zip routes actually stage before streaming.
+    res.headers = {
+      'Content-Length': '1234',
+      ETag: '"x"',
+      'Content-Type': 'image/jpeg',
+      'Content-Disposition': 'attachment; filename="gallery.zip"',
+      'Cache-Control': 'private, max-age=1800',
+    };
+    const stream = fs.createReadStream(path.join(os.tmpdir(), `gone-${Date.now()}.jpg`));
+
+    pipeStreamToResponse(stream, res);
+    await settle();
+
+    expect(res.headers['Content-Length']).toBeUndefined();
+    expect(res.headers.ETag).toBeUndefined();
+    // Express does NOT overwrite an existing Content-Type, so leaving it makes
+    // res.json() emit JSON labelled image/jpeg — or a corrupt .zip download.
+    expect(res.headers['Content-Type']).toBeUndefined();
+    expect(res.headers['Content-Disposition']).toBeUndefined();
+  });
+
+  it('does not let a transient 404 be cached as a broken tile', async () => {
+    const res = makeRes();
+    // The thumbnail route stages 30 minutes; the hero route an hour.
+    res.headers = { 'Cache-Control': 'private, max-age=1800' };
+    const stream = fs.createReadStream(path.join(os.tmpdir(), `gone3-${Date.now()}.jpg`));
+
+    pipeStreamToResponse(stream, res);
+    await settle();
+
+    // The regeneration race is transient by definition: the tier exists moments
+    // later. Caching this 404 would keep the tile broken long after the file is
+    // back — the opposite of what this helper is for.
+    expect(res.headers['Cache-Control']).toBe('no-store');
+  });
+
+  it('honours a caller that wants a different missing-status', async () => {
+    const res = makeRes();
+    const stream = fs.createReadStream(path.join(os.tmpdir(), `gone2-${Date.now()}.zip`));
+
+    pipeStreamToResponse(stream, res, { missingStatus: 410 });
+    await settle();
+
+    expect(res.statusCode).toBe(410);
+  });
+
+  it('destroys the response instead of rewriting a status that is already sent', async () => {
+    const res = makeRes();
+    res.headersSent = true;
+
+    const stream = new Readable({ read() {} });
+    pipeStreamToResponse(stream, res, { context: 'photo 9' });
+    stream.emit('error', Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
+    await settle();
+
+    // Once bytes are on the wire a 404 is not available; a truncated image the
+    // client would cache is worse than a broken connection.
+    expect(res.destroyed).toBe(true);
+    expect(res.statusCode).toBe(200);
+    expect(res.body).toBeNull();
+  });
+
+  it('reports a non-ENOENT failure as a 500 rather than a 404', async () => {
+    const res = makeRes();
+    const stream = new Readable({ read() {} });
+
+    pipeStreamToResponse(stream, res);
+    stream.emit('error', Object.assign(new Error('disk exploded'), { code: 'EIO' }));
+    await settle();
+
+    expect(res.statusCode).toBe(500);
+    expect(res.body).toEqual({ error: 'Failed to serve file' });
+  });
+
+  it('releases the source when the client hangs up mid-download', async () => {
+    const res = makeRes();
+    let destroyed = false;
+    const stream = new Readable({ read() {}, destroy(err, cb) { destroyed = true; cb(err); } });
+
+    pipeStreamToResponse(stream, res);
+    res.emit('close');
+    await settle();
+
+    // Otherwise an abandoned grid leaks one open fd per tile.
+    expect(destroyed).toBe(true);
+  });
+});

backend/src/routes/gallery.js

@@ -29,6 +29,7 @@ const { resolveGuest } = require('../middleware/guestAuth');
 const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit');
 const secureImageService = require('../services/secureImageService');
 const logger = require('../utils/logger');
+const { pipeStreamToResponse } = require('../utils/streamResponse');
 const { resolvePhotoFilePath } = require('../services/photoResolver');
 const { getEventShareToken, resolveShareIdentifier, buildShareLinkVariants } = require('../services/shareLinkService');
 const { handleAsync, errorResponse } = require('../utils/routeHelpers');
@@ -1044,7 +1045,7 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
       res.setHeader('Content-Length', zipInfo.size);
       res.setHeader('Content-Disposition', `attachment; filename="${req.event.slug}.zip"`);
       const stream = await storage.get(zipInfo.key);
-      stream.pipe(res);
+      pipeStreamToResponse(stream, res, { context: `prepared zip for event ${req.event.id}`, missingStatus: 410 });
 
       // Log bulk download
       db('access_logs').insert({
@@ -1505,7 +1506,7 @@ router.get('/:slug/photo/:photoId',
           const file = useStorageBackend
             ? await storage.getRange(storageKey, start, end)
             : fs.createReadStream(filePath, { start, end });
-          file.pipe(res);
+          pipeStreamToResponse(file, res, { context: `video range for photo ${photo.id}` });
         } else {
           res.writeHead(200, {
             'Content-Length': fileSize,
@@ -1517,7 +1518,7 @@ router.get('/:slug/photo/:photoId',
           const file = useStorageBackend
             ? await storage.get(storageKey)
             : fs.createReadStream(filePath);
-          file.pipe(res);
+          pipeStreamToResponse(file, res, { context: `video for photo ${photo.id}` });
         }
         return;
       }
@@ -1551,7 +1552,7 @@ router.get('/:slug/photo/:photoId',
                   'X-Protection-Level': 'basic'
                 });
                 const wmStream = await storage.get(photo.watermark_path);
-                return wmStream.pipe(res);
+                return pipeStreamToResponse(wmStream, res, { context: `watermarked photo ${photo.id}` });
               }
             } else {
               const watermarkFilePath = path.join(getStoragePath(), photo.watermark_path);
@@ -1600,7 +1601,7 @@ router.get('/:slug/photo/:photoId',
           res.set('Content-Length', stat.size);
           if (photo.mime_type) res.set('Content-Type', photo.mime_type);
           const stream = await storage.get(storageKey);
-          stream.pipe(res);
+          pipeStreamToResponse(stream, res, { context: `photo ${photo.id}` });
         } else {
           const absolutePath = path.isAbsolute(filePath) ? filePath : path.resolve(filePath);
           res.sendFile(absolutePath);
@@ -1695,7 +1696,7 @@ router.get('/:slug/thumbnail/:photoId',
       } else {
         res.setHeader('Content-Length', stat.size);
         const stream = await storage.get(thumbnailPath);
-        stream.pipe(res);
+        pipeStreamToResponse(stream, res, { context: `thumbnail for photo ${photoId}` });
       }
     } catch (error) {
       errorResponse(res, error, 500, 'Failed to serve thumbnail');
@@ -1781,7 +1782,7 @@ router.get('/:slug/hero/:photoId',
       } else {
         res.setHeader('Content-Length', stat.size);
         const stream = await storage.get(heroPath);
-        stream.pipe(res);
+        pipeStreamToResponse(stream, res, { context: `hero for photo ${photoId}` });
       }
     } catch (error) {
       logger.error('Error serving hero image:', {
@@ -1877,7 +1878,7 @@ router.get('/:slug/preview/:photoId',
       } else {
         res.setHeader('Content-Length', stat.size);
         const stream = await storage.get(previewPath);
-        stream.pipe(res);
+        pipeStreamToResponse(stream, res, { context: `preview for photo ${photoId}` });
       }
     } catch (error) {
       logger.error('Error serving preview image:', {

backend/src/utils/streamResponse.js

@@ -0,0 +1,81 @@
+/**
+ * Pipe a file/storage stream to an Express response without betting the
+ * process on the source still being there (#1128).
+ *
+ * `fs.createReadStream` — what LocalFsStorage.get() returns — is LAZY. It
+ * resolves immediately and only opens the file on a later tick, so an ENOENT
+ * arrives AFTER the `await` returned and outside the route's try/catch. An
+ * EventEmitter that emits 'error' with no listener throws, and an uncaught
+ * throw from an I/O callback is not something Express can catch: Node exits.
+ *
+ * That is how one missing thumbnail tier took down every gallery on the
+ * install — the process died on the first grid load and only came back
+ * because Docker restarted it.
+ *
+ * The window is real and cannot be closed by a stat() beforehand: between the
+ * stat and the open, another request regenerating the same derivative can
+ * unlink it. So the handler is the fix, not the preflight.
+ */
+
+const logger = require('./logger');
+
+/**
+ * @param {import('stream').Readable} stream  source, already opened or lazy
+ * @param {import('express').Response} res
+ * @param {object}  [options]
+ * @param {string}  [options.context]  what was being served, for the log line
+ * @param {number}  [options.missingStatus=404]  status when the source is gone
+ */
+function pipeStreamToResponse(stream, res, options = {}) {
+  const { context = 'file', missingStatus = 404 } = options;
+
+  stream.on('error', (err) => {
+    const gone = err && (err.code === 'ENOENT' || err.code === 'EISDIR');
+
+    // Once bytes are on the wire the status line is spent — there is no way to
+    // turn this into a 404. Destroy the response so the client sees a broken
+    // connection rather than a silently truncated image it would cache.
+    if (res.headersSent) {
+      logger.warn(`Stream failed mid-response for ${context}: ${err.message}`);
+      res.destroy(err);
+      return;
+    }
+
+    // Every header staged for the FILE now describes a body that will never
+    // be sent. They are cleared rather than left to Express, which does not
+    // overwrite a Content-Type that is already set — so without this the JSON
+    // error goes out as `image/jpeg`, or as an `application/zip` attachment
+    // that saves to disk as a corrupt download.
+    //
+    // Cache-Control matters most. The image routes stage `max-age=1800` (the
+    // hero route 3600), so a 404 from the regeneration race — the transient
+    // case this whole helper exists for — would be cached as a broken tile for
+    // up to an hour after the tier finished generating.
+    res.removeHeader('Content-Length');
+    res.removeHeader('ETag');
+    res.removeHeader('Content-Type');
+    res.removeHeader('Content-Disposition');
+    res.setHeader('Cache-Control', 'no-store');
+
+    if (gone) {
+      // Expected under the regeneration race — the tier existed at stat time
+      // and was replaced before the open. One broken tile, not an outage.
+      logger.warn(`Source vanished while serving ${context}: ${err.message}`);
+      res.status(missingStatus).json({ error: 'File not found' });
+      return;
+    }
+
+    logger.error(`Failed to stream ${context}`, { error: err.message, code: err.code });
+    res.status(500).json({ error: 'Failed to serve file' });
+  });
+
+  // A client that navigates away mid-download leaves the source handle open
+  // otherwise; on a gallery grid that is one leaked fd per abandoned tile.
+  res.on('close', () => {
+    if (!res.writableEnded) stream.destroy();
+  });
+
+  stream.pipe(res);
+}
+
+module.exports = { pipeStreamToResponse };