From 247145f5454ffc0518e4a52aa53eb78e7d45c383 Mon Sep 17 00:00:00 2001 From: Hiren Kangad Date: Sat, 14 Mar 2026 11:16:58 +0530 Subject: [PATCH] fix: early PDF validation, type-aware errors, exact job dedup - Move PDF page count check before MinIO upload and DB record creation to prevent orphaned objects when >500 page PDFs are rejected - Use job type label (PDF page / Video) in media-worker error messages instead of hardcoded "Video processing failed" - Replace LIKE-based idempotency check with exact JSON match to prevent page 1 matching page 11/12/etc substring collisions --- backend/routes/pdf.js | 8 ++-- backend/routes/upload.js | 73 ++++++++++++++++++++------------ backend/services/media-worker.js | 9 ++-- 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/backend/routes/pdf.js b/backend/routes/pdf.js index 7ddcdde..2be1dbe 100644 --- a/backend/routes/pdf.js +++ b/backend/routes/pdf.js @@ -67,8 +67,8 @@ router.post('/:boardId/pdf-pages', async (req, res) => { // Check for existing pending/processing hires job for this page const { db } = require('../db'); const existingJob = db.prepare( - `SELECT id FROM media_jobs WHERE image_id = ? AND type = 'pdf-hires' AND result_json LIKE ? AND status IN ('queued', 'processing')` - ).get(imageId, `%"pageNumber":${pageNum}%`); + `SELECT id FROM media_jobs WHERE image_id = ? AND type = 'pdf-hires' AND result_json = ? AND status IN ('queued', 'processing')` + ).get(imageId, JSON.stringify({ pageNumber: pageNum })); if (!existingJob) { createMediaJob({ @@ -126,8 +126,8 @@ router.post('/:boardId/pdf-thumbnails', async (req, res) => { // Skip if job already pending const { db } = require('../db'); const existingJob = db.prepare( - `SELECT id FROM media_jobs WHERE image_id = ? AND type = 'pdf-thumbnail' AND result_json LIKE ? AND status IN ('queued', 'processing')` - ).get(imageId, `%"pageNumber":${pageNum}%`); + `SELECT id FROM media_jobs WHERE image_id = ? AND type = 'pdf-thumbnail' AND result_json = ? AND status IN ('queued', 'processing')` + ).get(imageId, JSON.stringify({ pageNumber: pageNum })); if (!existingJob) { createMediaJob({ diff --git a/backend/routes/upload.js b/backend/routes/upload.js index 88a6a68..85d5a7b 100644 --- a/backend/routes/upload.js +++ b/backend/routes/upload.js @@ -131,33 +131,7 @@ router.post('/boards/:boardId/images', upload.single('image'), async (req, res) const { buffer, originalname, mimetype, size } = req.file; const mediaType = classifyMedia(mimetype); - const { assetKey, minioPath, width, height, isVideo } = await uploadMedia(board.id, imageId, buffer, mimetype); - const publicUrl = getImageUrl(minioPath); - - // Save image record first (media_jobs FK references images) - const image = createImage({ - id: imageId, - boardId: board.id, - filename: originalname, - mimeType: mimetype, - fileSize: size, - width, - height, - minioPath, - publicUrl, - uploadedBy: req.user.id, - assetKey, - mediaType, - }); - - // Enqueue background processing after image record exists - let jobId = null; - if (isVideo) { - jobId = uuidv4(); - createMediaJob({ id: jobId, imageId, boardId: board.id, type: 'poster' }); - } - - // PDF: extract page info, create pdf_pages rows, queue thumbnail jobs + // PDF: validate page count BEFORE uploading to MinIO or creating DB records if (mediaType === 'pdf') { const { tmpPath, cleanup } = bufferToTempFile(buffer, '.pdf'); try { @@ -166,6 +140,25 @@ router.post('/boards/:boardId/images', upload.single('image'), async (req, res) return res.status(400).json({ error: 'PDF exceeds 500 page limit' }); } + // Validation passed — now upload and create records + const { assetKey, minioPath, width, height } = await uploadMedia(board.id, imageId, buffer, mimetype); + const publicUrl = getImageUrl(minioPath); + + const image = createImage({ + id: imageId, + boardId: board.id, + filename: originalname, + mimeType: mimetype, + fileSize: size, + width, + height, + minioPath, + publicUrl, + uploadedBy: req.user.id, + assetKey, + mediaType, + }); + updateImagePageCount(imageId, info.pageCount); // Create pdf_pages rows for all pages @@ -214,6 +207,32 @@ router.post('/boards/:boardId/images', upload.single('image'), async (req, res) } } + const { assetKey, minioPath, width, height, isVideo } = await uploadMedia(board.id, imageId, buffer, mimetype); + const publicUrl = getImageUrl(minioPath); + + // Save image record first (media_jobs FK references images) + const image = createImage({ + id: imageId, + boardId: board.id, + filename: originalname, + mimeType: mimetype, + fileSize: size, + width, + height, + minioPath, + publicUrl, + uploadedBy: req.user.id, + assetKey, + mediaType, + }); + + // Enqueue background processing after image record exists + let jobId = null; + if (isVideo) { + jobId = uuidv4(); + createMediaJob({ id: jobId, imageId, boardId: board.id, type: 'poster' }); + } + return res.status(201).json({ id: image.id, url: publicUrl, diff --git a/backend/services/media-worker.js b/backend/services/media-worker.js index fb4a02e..1841775 100644 --- a/backend/services/media-worker.js +++ b/backend/services/media-worker.js @@ -144,13 +144,14 @@ async function processJob(job) { console.error('[media-worker] Job %s failed:', jobId, err.message); // Classify error for user-facing message - let userError = 'Video processing failed'; + const typeLabel = job.type.startsWith('pdf-') ? 'PDF page' : 'Video'; + let userError = `${typeLabel} processing failed`; if (err.killed || err.signal === 'SIGTERM') { - userError = 'Video processing timed out — file may be too large or corrupt'; + userError = `${typeLabel} processing timed out — file may be too large or corrupt`; } else if (err.message?.includes('ENOMEM') || err.message?.includes('Cannot allocate')) { - userError = 'Out of memory — video file is too large to process'; + userError = `Out of memory — file is too large to process`; } else if (err.message?.includes('Invalid data')) { - userError = 'Invalid or corrupt video file'; + userError = `Invalid or corrupt ${typeLabel.toLowerCase()} file`; } const attempts = (job.attempts || 0) + 1;