From 852bcd1133aa26cabf06be5477459e262a4a31e5 Mon Sep 17 00:00:00 2001
From: Paul Nothaft <paul@MacStudio-von-Paul.local>
Date: Thu, 10 Sep 2026 13:40:39 +0200
Subject: [PATCH] fix(video): throw when neither a real thumbnail nor the
 placeholder can be produced

processUploadedVideo returned success with thumbnailKey: null when
both the real thumbnail AND the SVG placeholder failed -- a total,
systemic failure (storage backend down, disk full), not a quirk of
one file. On stable, which doesn't have the #845 call-site fallback,
this silently completed the video with no thumbnail at all instead
of the retryable 'failed' status a throw here produces. On main,
the pre-existing #845 fallback already absorbed this exact case
(no behavior change there) -- verified against codex's own
git-blame check of the pre-PR stable code before applying this.

Now throws in that case, restoring the pre-existing "let the caller
mark it failed and retryable" behavior for a genuinely unrecoverable
video, while keeping every partial-failure case (the vast majority)
resolving with whatever succeeded.

Found by codex review.
---
 .../services/__tests__/videoProcessor.test.js |  7 +++---
 backend/src/services/videoProcessor.js        | 24 ++++++++++++++++---
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/backend/src/services/__tests__/videoProcessor.test.js b/backend/src/services/__tests__/videoProcessor.test.js
index ec88190c..a6638b1d 100644
--- a/backend/src/services/__tests__/videoProcessor.test.js
+++ b/backend/src/services/__tests__/videoProcessor.test.js
@@ -108,7 +108,7 @@ describe('processUploadedVideo degrades gracefully instead of rejecting the whol
     expect(storage.putFromFile).not.toHaveBeenCalled();
   });
 
-  it('still resolves with a null thumbnail when metadata, thumbnail generation, AND the placeholder all fail — never throws, never blocks the upload', async () => {
+  it('throws when metadata, thumbnail generation, AND the placeholder all fail, so the caller surfaces a retryable failure instead of completing with nothing to show (codex review)', async () => {
     ffmpeg.ffprobe = jest.fn((videoPath, cb) => cb(new Error('Invalid data found when processing input')));
     ffmpeg.mockImplementation(() => ({
       screenshots() { return this; },
@@ -119,8 +119,7 @@ describe('processUploadedVideo degrades gracefully instead of rejecting the whol
     }));
     generateVideoPlaceholder.mockRejectedValue(new Error('sharp render failed'));
 
-    const result = await processUploadedVideo('/tmp/corrupt.mp4', 'thumbnails/thumb_corrupt.jpg');
-
-    expect(result).toEqual({ success: true, metadata: null, thumbnailKey: null });
+    await expect(processUploadedVideo('/tmp/corrupt.mp4', 'thumbnails/thumb_corrupt.jpg'))
+      .rejects.toThrow('Unable to generate any thumbnail');
   });
 });
diff --git a/backend/src/services/videoProcessor.js b/backend/src/services/videoProcessor.js
index 21182532..eedad31d 100644
--- a/backend/src/services/videoProcessor.js
+++ b/backend/src/services/videoProcessor.js
@@ -145,12 +145,18 @@ async function getVideoDuration(videoPath) {
  * generateVideoThumbnail() would have succeeded on its own — thumbnailing
  * doesn't need valid duration/width/height, it just seeks and grabs a frame.
  * Trying both steps independently means a real thumbnail (and whatever
- * metadata ffprobe *can* read) survives far more often; the callers' throw
- * handling stays as a backstop for anything still unexpected.
+ * metadata ffprobe *can* read) survives far more often. metadata is still
+ * allowed to come back null (ffprobe failed) — a video with no thumbnail
+ * would fall back to rendering the raw video as an <img> in the gallery
+ * grid (`photo.thumbnail_url || photo.url`), so this only resolves when a
+ * real thumbnail or the SVG placeholder produced *something*; if both fail
+ * (storage backend down, disk full — not a quirk of one file) it throws
+ * instead, so the caller surfaces a retryable failure rather than silently
+ * completing with nothing to show.
  *
  * @param {string} videoPath - Local path to the source video (ffmpeg requires fs).
  * @param {string} thumbnailKey - Relative storage key for the thumbnail.
- * @returns {Promise<{success: boolean, metadata: Object|null, thumbnailKey: string|null}>}
+ * @returns {Promise<{success: boolean, metadata: Object|null, thumbnailKey: string}>}
  */
 async function processUploadedVideo(videoPath, thumbnailKey, options = {}) {
   let metadata = null;
@@ -212,6 +218,18 @@ async function processUploadedVideo(videoPath, thumbnailKey, options = {}) {
     }
   }
 
+  // A real thumbnail AND the ffmpeg-free SVG placeholder both failing points
+  // at something systemic (storage backend down, disk full) rather than a
+  // quirk of this one file — that's worth surfacing as a retryable failure
+  // rather than silently completing with no thumbnail at all, which would
+  // make the gallery fall back to rendering the raw video as an <img>
+  // (codex review, #1371/#1372). Metadata (if any was extracted) is lost
+  // here, same trade-off the callers' own pre-existing total-failure
+  // handling already makes.
+  if (!generatedThumbnailKey) {
+    throw new Error('Unable to generate any thumbnail (real or placeholder) for this video');
+  }
+
   return {
     success: true,
     metadata,