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.

backend/__tests__/services/videoPlaceholderNoDbLookup.test.js

@@ -0,0 +1,62 @@
+/**
+ * generateVideoPlaceholder() must not touch the database when the caller
+ * already supplies width/height (videoProcessor.js's thumbnail-generation
+ * fallback does exactly this).
+ *
+ * Why it matters: processUploadedPhotos() (chunked video upload) holds a
+ * per-file SQLite transaction open across thumbnail generation. SQLite's
+ * knex pool defaults to a single connection, so any second, un-transacted
+ * db() query made while that transaction is open blocks until
+ * acquireConnectionTimeout (60s in production) — verified directly against
+ * an isolated SQLite db (codex review of #1371/#1372). Passing explicit
+ * dimensions must skip getThumbnailSettings()'s db() call entirely, not
+ * just tolerate its failure.
+ */
+
+const path = require('path');
+const fs = require('fs').promises;
+const os = require('os');
+
+const mockDbSpy = jest.fn(() => {
+  throw new Error('db() must not be called when width/height are supplied');
+});
+jest.mock('../../src/database/db', () => ({ db: (...args) => mockDbSpy(...args) }));
+
+const storageModule = require('../../src/services/storage');
+const LocalFsStorage = require('../../src/services/storage/LocalFsStorage');
+
+describe('generateVideoPlaceholder skips the settings DB lookup given explicit dimensions', () => {
+  let storage;
+  let root;
+  let imageProcessor;
+
+  beforeAll(async () => {
+    root = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-vidplaceholder-'));
+    storage = new LocalFsStorage({ root });
+    await storage.init();
+    storageModule.setStorageForTesting(storage);
+    imageProcessor = require('../../src/services/imageProcessor');
+  }, 30000);
+
+  afterAll(async () => {
+    storageModule.resetStorage();
+    await fs.rm(root, { recursive: true, force: true }).catch(() => {});
+  });
+
+  afterEach(() => mockDbSpy.mockClear());
+
+  it('never calls db() when width/height are provided', async () => {
+    const key = await imageProcessor.generateVideoPlaceholder('demo.mp4', { width: 300, height: 300 });
+
+    expect(key).toBe('thumbnails/thumb_demo.jpg');
+    expect(await storage.exists(key)).toBe(true);
+    expect(mockDbSpy).not.toHaveBeenCalled();
+  });
+
+  it('falls through to defaults (not a throw) when db() fails and no dimensions were given', async () => {
+    const key = await imageProcessor.generateVideoPlaceholder('demo2.mp4');
+
+    expect(key).toBe('thumbnails/thumb_demo2.jpg');
+    expect(mockDbSpy).toHaveBeenCalled();
+  });
+});

backend/src/services/__tests__/videoProcessor.test.js

@@ -0,0 +1,125 @@
+jest.mock('../../utils/logger');
+jest.mock('fluent-ffmpeg');
+jest.mock('../storage', () => ({
+  getStorage: jest.fn()
+}));
+jest.mock('../imageProcessor', () => ({
+  generateVideoPlaceholder: jest.fn(),
+  DEFAULT_THUMBNAIL_WIDTH: 300,
+  DEFAULT_THUMBNAIL_HEIGHT: 300
+}));
+
+const ffmpeg = require('fluent-ffmpeg');
+const { getStorage } = require('../storage');
+const { generateVideoPlaceholder } = require('../imageProcessor');
+const {
+  extractVideoMetadata,
+  processUploadedVideo
+} = require('../videoProcessor');
+
+describe('extractVideoMetadata (#1370)', () => {
+  afterEach(() => jest.clearAllMocks());
+
+  it('returns null duration rather than 0 when ffprobe has none, so "unknown" and "a real 0s clip" stay distinguishable', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
+      cb(null, {
+        streams: [{ codec_type: 'video', width: 1920, height: 1080, codec_name: 'hevc' }],
+        format: {} // no duration field at all
+      });
+    });
+
+    const metadata = await extractVideoMetadata('/tmp/video.mp4');
+
+    expect(metadata.duration).toBeNull();
+    expect(metadata.width).toBe(1920);
+    expect(metadata.videoCodec).toBe('hevc');
+  });
+
+  it('floors a real duration', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
+      cb(null, { streams: [], format: { duration: 12.9 } });
+    });
+
+    const metadata = await extractVideoMetadata('/tmp/video.mp4');
+
+    expect(metadata.duration).toBe(12);
+  });
+});
+
+describe('processUploadedVideo degrades gracefully instead of rejecting the whole video (#1370)', () => {
+  let storage;
+
+  beforeEach(() => {
+    storage = { putFromFile: jest.fn().mockResolvedValue(undefined), exists: jest.fn().mockResolvedValue(true) };
+    getStorage.mockReturnValue(storage);
+    generateVideoPlaceholder.mockResolvedValue('thumbnails/thumb_placeholder.jpg');
+  });
+
+  afterEach(() => jest.clearAllMocks());
+
+  it('keeps the thumbnail when only metadata extraction fails', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => cb(new Error('moov atom not found')));
+    ffmpeg.mockImplementation(() => ({
+      screenshots: jest.fn(function screenshots({ filename, folder }) {
+        require('fs').writeFileSync(require('path').join(folder, filename), 'jpeg-bytes');
+        return this;
+      }),
+      on(event, handler) {
+        if (event === 'end') setImmediate(handler);
+        return this;
+      }
+    }));
+
+    const result = await processUploadedVideo('/tmp/video.mp4', 'thumbnails/thumb_video.jpg');
+
+    expect(result.success).toBe(true);
+    expect(result.metadata).toBeNull();
+    expect(result.thumbnailKey).toBe('thumbnails/thumb_video.jpg');
+    // A real thumbnail already succeeded — never touch the placeholder path.
+    expect(generateVideoPlaceholder).not.toHaveBeenCalled();
+  });
+
+  it('falls back to the SVG placeholder when thumbnail generation fails, so the gallery never falls back to rendering the raw video as an <img> (codex review)', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
+      cb(null, {
+        streams: [{ codec_type: 'video', width: 1080, height: 1920, codec_name: 'h264' }],
+        format: { duration: 5.4 }
+      });
+    });
+    ffmpeg.mockImplementation(() => ({
+      screenshots() { return this; },
+      on(event, handler) {
+        if (event === 'error') setImmediate(() => handler(new Error('ffmpeg seek failed')));
+        return this;
+      }
+    }));
+
+    const result = await processUploadedVideo('/tmp/video.mp4', 'thumbnails/thumb_wedding_001.jpg');
+
+    expect(result.success).toBe(true);
+    expect(result.metadata).toEqual(expect.objectContaining({ duration: 5, videoCodec: 'h264' }));
+    // thumbnailKey is always thumbnails/thumb_<name>.jpg — strip the prefix
+    // back to a filename so generateVideoPlaceholder recomputes the same key.
+    // Explicit width/height so generateVideoPlaceholder skips its DB-backed
+    // settings lookup — this can run inside an open per-file SQLite
+    // transaction (chunked video upload), where that lookup deadlocks.
+    expect(generateVideoPlaceholder).toHaveBeenCalledWith('wedding_001.jpg', { width: 300, height: 300 });
+    expect(result.thumbnailKey).toBe('thumbnails/thumb_placeholder.jpg');
+    expect(storage.putFromFile).not.toHaveBeenCalled();
+  });
+
+  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; },
+      on(event, handler) {
+        if (event === 'error') setImmediate(() => handler(new Error('ffmpeg seek failed')));
+        return this;
+      }
+    }));
+    generateVideoPlaceholder.mockRejectedValue(new Error('sharp render failed'));
+
+    await expect(processUploadedVideo('/tmp/corrupt.mp4', 'thumbnails/thumb_corrupt.jpg'))
+      .rejects.toThrow('Unable to generate any thumbnail');
+  });
+});

backend/src/services/imageProcessor.js

@@ -610,9 +610,15 @@ async function generateVideoPlaceholder(originalFilename, options = {}) {
   const thumbnailRelKey = path.posix.join('thumbnails', thumbnailFilename);
   const storage = getStorage();
 
-  const settings = await getThumbnailSettings();
-  const width = settings.width || DEFAULT_THUMBNAIL_WIDTH;
-  const height = settings.height || DEFAULT_THUMBNAIL_HEIGHT;
+  // Skip the settings lookup when the caller already supplies dimensions.
+  // This can run from inside an open per-file SQLite transaction (chunked
+  // video upload's fallback path in videoProcessor.js) — a second,
+  // un-transacted db() query for settings there deadlocks against SQLite's
+  // single-connection pool until acquireConnectionTimeout (60s), reproduced
+  // directly against an isolated SQLite db (codex review of #1371/#1372).
+  const settings = (options.width && options.height) ? {} : await getThumbnailSettings();
+  const width = options.width || settings.width || DEFAULT_THUMBNAIL_WIDTH;
+  const height = options.height || settings.height || DEFAULT_THUMBNAIL_HEIGHT;
 
   if (options.regenerate) {
     await storage.delete(thumbnailRelKey).catch(() => {});
@@ -1495,4 +1501,6 @@ module.exports = {
   extractRawPreview,
   withProcessableImage,
   RAW_EXTENSIONS,
+  DEFAULT_THUMBNAIL_WIDTH,
+  DEFAULT_THUMBNAIL_HEIGHT,
 };

backend/src/services/videoProcessor.js

@@ -32,7 +32,10 @@ async function extractVideoMetadata(videoPath) {
         const audioStream = metadata.streams.find(s => s.codec_type === 'audio');
 
         const result = {
-          duration: Math.floor(metadata.format.duration || 0),
+          // null (not 0) when ffprobe genuinely has no duration — a real
+          // 0-second clip and "unknown" must stay distinguishable, since
+          // downstream code treats `duration != null` as "trust this value".
+          duration: metadata.format.duration != null ? Math.floor(metadata.format.duration) : null,
           width: videoStream?.width || null,
           height: videoStream?.height || null,
           videoCodec: videoStream?.codec_name || null,
@@ -130,35 +133,108 @@ async function getVideoDuration(videoPath) {
  * Process an uploaded video: extract metadata and produce a thumbnail through
  * the storage backend.
  *
+ * Metadata extraction and thumbnail generation are independent, best-effort
+ * steps — mirroring how the image pipeline treats thumbnail/dimension/EXIF
+ * failures (log a warning, keep the upload). This used to gate everything
+ * behind isValidVideo(), which rejects the whole video if ffprobe can't read
+ * even one of duration/width/height — common on some iPhone/Lightroom-
+ * exported MP4s (#1370). Callers (photoProcessor.js's processPhoto and
+ * processUploadedPhotos) already catch that throw and fall back to a static
+ * placeholder thumbnail plus a metadata-only retry (codex review of #845),
+ * but that fallback never got a REAL thumbnail even when
+ * 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. 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, thumbnailKey: string}>}
+ * @returns {Promise<{success: boolean, metadata: Object|null, thumbnailKey: string}>}
  */
 async function processUploadedVideo(videoPath, thumbnailKey, options = {}) {
+  let metadata = null;
   try {
-    const isValid = await isValidVideo(videoPath);
-    if (!isValid) {
-      throw new Error('Invalid video file');
-    }
-
-    const metadata = await extractVideoMetadata(videoPath);
-    await generateVideoThumbnail(videoPath, thumbnailKey, options);
-
-    const storage = getStorage();
-    const exists = await storage.exists(thumbnailKey);
-    if (!exists) {
-      throw new Error('Thumbnail generation failed (not in storage)');
-    }
-
-    return {
-      success: true,
-      metadata,
-      thumbnailKey
-    };
+    metadata = await extractVideoMetadata(videoPath);
   } catch (error) {
-    logger.error('Error processing video', { error: error.message, videoPath });
-    throw error;
+    logger.error('Video metadata extraction failed — continuing without duration/codec/dimensions', {
+      error: error.message,
+      videoPath
+    });
   }
+
+  let generatedThumbnailKey = null;
+  try {
+    await generateVideoThumbnail(videoPath, thumbnailKey, options);
+    const storage = getStorage();
+    if (await storage.exists(thumbnailKey)) {
+      generatedThumbnailKey = thumbnailKey;
+    }
+  } catch (error) {
+    logger.error('Video thumbnail generation failed — continuing without a thumbnail', {
+      error: error.message,
+      videoPath
+    });
+  }
+
+  // Never return "success" with no thumbnail at all: the gallery grid
+  // (GridGalleryLayout/JustifiedGalleryLayout) falls back to
+  // `photo.thumbnail_url || photo.url` when there's no thumbnail, which
+  // makes AuthenticatedImage download the full ORIGINAL VIDEO and try to
+  // render it as an <img> — a broken tile and a multi-GB fetch just from
+  // opening the gallery (codex review, #1371/#1372). Fall back to the same
+  // ffmpeg-free SVG placeholder the callers already generate for a total
+  // processing failure, so a bare thumbnail-generation failure degrades to
+  // that placeholder too, not to "no thumbnail". thumbnailKey is always
+  // `thumbnails/thumb_<name>.jpg` (see callers) — strip the prefix back to
+  // a filename so generateVideoPlaceholder recomputes this exact same key.
+  if (!generatedThumbnailKey) {
+    try {
+      const {
+        generateVideoPlaceholder,
+        DEFAULT_THUMBNAIL_WIDTH,
+        DEFAULT_THUMBNAIL_HEIGHT
+      } = require('./imageProcessor');
+      const placeholderFilename = path.basename(thumbnailKey).replace(/^thumb_/, '');
+      // Explicit width/height make generateVideoPlaceholder skip its
+      // configured-thumbnail-size DB lookup (see its own comment) — this
+      // call can run from inside processUploadedPhotos' open per-file
+      // SQLite transaction, where that lookup would otherwise deadlock.
+      const placeholderKey = await generateVideoPlaceholder(placeholderFilename, {
+        width: DEFAULT_THUMBNAIL_WIDTH,
+        height: DEFAULT_THUMBNAIL_HEIGHT
+      });
+      if (placeholderKey) {
+        generatedThumbnailKey = placeholderKey;
+      }
+    } catch (error) {
+      logger.error('Video placeholder generation also failed', { error: error.message, videoPath });
+    }
+  }
+
+  // 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,
+    thumbnailKey: generatedThumbnailKey
+  };
 }
 
 /**