fix(video): fall back to the SVG placeholder when thumbnail generation fails

processUploadedVideo could return success with thumbnailKey: null
when only thumbnail generation failed. The gallery grid
(GridGalleryLayout/JustifiedGalleryLayout) falls back to
`photo.thumbnail_url || photo.url` when there's no thumbnail, so
AuthenticatedImage downloaded the full original video and tried to
render it as an <img> -- a broken tile and a potentially huge
fetch just from opening the gallery.

Falls 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,
never to "no thumbnail at all".

Found by codex review.
This commit is contained in:
Paul Nothaft
2026-09-10 13:16:58 +02:00
parent 30c3134891
commit 69436fba91
2 changed files with 39 additions and 4 deletions
@@ -3,9 +3,13 @@ jest.mock('fluent-ffmpeg');
jest.mock('../storage', () => ({
getStorage: jest.fn()
}));
jest.mock('../imageProcessor', () => ({
generateVideoPlaceholder: jest.fn()
}));
const ffmpeg = require('fluent-ffmpeg');
const { getStorage } = require('../storage');
const { generateVideoPlaceholder } = require('../imageProcessor');
const {
extractVideoMetadata,
processUploadedVideo
@@ -46,6 +50,7 @@ describe('processUploadedVideo degrades gracefully instead of rejecting the whol
beforeEach(() => {
storage = { putFromFile: jest.fn().mockResolvedValue(undefined), exists: jest.fn().mockResolvedValue(true) };
getStorage.mockReturnValue(storage);
generateVideoPlaceholder.mockResolvedValue('thumbnails/thumb_placeholder.jpg');
});
afterEach(() => jest.clearAllMocks());
@@ -68,9 +73,11 @@ describe('processUploadedVideo degrades gracefully instead of rejecting the whol
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('keeps the metadata when only thumbnail generation fails', async () => {
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' }],
@@ -85,15 +92,18 @@ describe('processUploadedVideo degrades gracefully instead of rejecting the whol
}
}));
const result = await processUploadedVideo('/tmp/video.mp4', 'thumbnails/thumb_video.jpg');
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' }));
expect(result.thumbnailKey).toBeNull();
// thumbnailKey is always thumbnails/thumb_<name>.jpg — strip the prefix
// back to a filename so generateVideoPlaceholder recomputes the same key.
expect(generateVideoPlaceholder).toHaveBeenCalledWith('wedding_001.jpg');
expect(result.thumbnailKey).toBe('thumbnails/thumb_placeholder.jpg');
expect(storage.putFromFile).not.toHaveBeenCalled();
});
it('still succeeds with both null when metadata AND thumbnail fail — never throws, never blocks the upload', async () => {
it('still resolves with a null thumbnail when metadata, thumbnail generation, AND the placeholder all fail — never throws, never blocks the upload', async () => {
ffmpeg.ffprobe = jest.fn((videoPath, cb) => cb(new Error('Invalid data found when processing input')));
ffmpeg.mockImplementation(() => ({
screenshots() { return this; },
@@ -102,6 +112,7 @@ describe('processUploadedVideo degrades gracefully instead of rejecting the whol
return this;
}
}));
generateVideoPlaceholder.mockRejectedValue(new Error('sharp render failed'));
const result = await processUploadedVideo('/tmp/corrupt.mp4', 'thumbnails/thumb_corrupt.jpg');
+24
View File
@@ -177,6 +177,30 @@ async function processUploadedVideo(videoPath, thumbnailKey, options = {}) {
});
}
// 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 } = require('./imageProcessor');
const placeholderFilename = path.basename(thumbnailKey).replace(/^thumb_/, '');
const placeholderKey = await generateVideoPlaceholder(placeholderFilename);
if (placeholderKey) {
generatedThumbnailKey = placeholderKey;
}
} catch (error) {
logger.error('Video placeholder generation also failed', { error: error.message, videoPath });
}
}
return {
success: true,
metadata,