D2: Fix upload retry 500 on missing file — async fire-and-forget
retryJob() now wraps the file read in a fire-and-forget async IIFE using fs/promises.readFile instead of readFileSync. When the original upload file has been cleaned up (common after failure), the IIFE catches the ENOENT error and marks the job as 'failed' with a 'retry failed:' prefix asynchronously — the endpoint returns 200 immediately. Includes 4 new integration tests: - Missing file returns immediately (no throw), job async-fails - ENOENT message propagated in job.error - 404 on unknown job still works (unchanged contract) - File present resets job to pending (runJob triggered)
This commit is contained in:
Giancarmine Salucci
@@ -50,7 +50,11 @@ export async function startUploadJob(
|
||||
return job.id;
|
||||
}
|
||||
|
||||
/** Retry a failed/cancelled job — YouTube or upload-source. */
|
||||
/** Retry a failed/cancelled job — YouTube or upload-source.
|
||||
*
|
||||
* Upload retry reads the original file asynchronously (fire-and-forget IIFE)
|
||||
* so a missing file produces an async job failure instead of a synchronous
|
||||
* 500 crash. The outer function returns immediately. */
|
||||
export async function retryJob(jobId: string): Promise<void> {
|
||||
const job = getJob(jobId);
|
||||
if (!job) throw new Error('Job not found');
|
||||
@@ -58,12 +62,22 @@ export async function retryJob(jobId: string): Promise<void> {
|
||||
|
||||
const isUpload = !job.source.startsWith('http');
|
||||
if (isUpload) {
|
||||
const { readFileSync } = await import('fs');
|
||||
// Fire-and-forget: missing file -> async job failure, not 500
|
||||
(async () => {
|
||||
try {
|
||||
const { readFile } = await import('fs/promises');
|
||||
const uploadPath = getUploadPath(jobId, job.source);
|
||||
const buffer = readFileSync(uploadPath);
|
||||
const buffer = await readFile(uploadPath);
|
||||
runJob(jobId, { type: 'upload', buffer, filename: job.source }, job.audioMode as AudioMode).catch((err) => {
|
||||
console.error(`[pipeline] retry upload job ${jobId} failed:`, err);
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
console.error(`[pipeline] retry upload job ${jobId} failed to read file:`, message);
|
||||
updateJob({ id: jobId, status: 'failed', error: `retry failed: ${message}` });
|
||||
emitProgress(jobId, { type: 'error', message: `retry failed: ${message}` });
|
||||
}
|
||||
})();
|
||||
} else {
|
||||
runJob(jobId, { type: 'youtube', url: job.source }, job.audioMode as AudioMode).catch((err) => {
|
||||
console.error(`[pipeline] retry job ${jobId} failed:`, err);
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
import { describe, it, expect, afterAll, vi } from 'vitest';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { rm } from 'fs/promises';
|
||||
|
||||
/**
|
||||
* Integration test for the retry endpoint against the REAL retryJob +
|
||||
* REAL SQLite DB (no pipeline mock). Covers the exact outcome-contract
|
||||
* scenario the mocked unit suite could not: retrying a failed upload-source
|
||||
* job whose original uploaded audio file is ABSENT on disk.
|
||||
*
|
||||
* The endpoint must still return 200 { ok: true } (retryJob resolves
|
||||
* immediately; the resubmit is fire-and-forget) and the missing file must
|
||||
* fail the JOB (status -> failed), not the HTTP request (which was a 500
|
||||
* regression before the fix).
|
||||
*/
|
||||
|
||||
// DATA_DIR must be stubbed before db.js AND downloader.js import (both read
|
||||
// it at module load to derive the jobs.db path and the uploads dir).
|
||||
const TEST_DATA_DIR = join(tmpdir(), 'whisper-pwa-retry-int-' + process.pid);
|
||||
vi.stubEnv('DATA_DIR', TEST_DATA_DIR);
|
||||
|
||||
const { createJob, getJob, updateJob } = await import('$lib/server/db.js');
|
||||
const { POST } = await import('$lib/../routes/api/jobs/[id]/retry/+server.js');
|
||||
|
||||
afterAll(async () => {
|
||||
await rm(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function makeEvent(jobId: string) {
|
||||
return { params: { id: jobId } } as any;
|
||||
}
|
||||
|
||||
async function waitForStatus(id: string, status: string, timeoutMs = 2000): Promise<void> {
|
||||
const start = Date.now();
|
||||
// eslint-disable-next-line no-constant-condition
|
||||
while (true) {
|
||||
const job = getJob(id);
|
||||
if (job && job.status === status) return;
|
||||
if (Date.now() - start > timeoutMs) {
|
||||
throw new Error(`timed out waiting for job ${id} status=${status}; got ${job?.status}`);
|
||||
}
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
}
|
||||
}
|
||||
|
||||
describe('POST /api/jobs/:id/retry — real retryJob (upload, missing file)', () => {
|
||||
it('returns 200 and fails the JOB (not the request) when the upload file is absent', async () => {
|
||||
// Failed upload-source job — non-http source, no file on disk.
|
||||
const job = createJob('my-recording.webm', 'Upload retry probe', 'auto');
|
||||
updateJob({ id: job.id, status: 'failed', error: 'original failure' });
|
||||
|
||||
// The HTTP request must stay 200 even though the file is gone.
|
||||
const res = await POST(makeEvent(job.id));
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.json()).toEqual({ ok: true });
|
||||
|
||||
// The missing file fails the job asynchronously.
|
||||
await waitForStatus(job.id, 'failed');
|
||||
const after = getJob(job.id)!;
|
||||
expect(after.status).toBe('failed');
|
||||
expect(after.error).toMatch(/retry failed/i);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,78 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { rm, mkdir, writeFile } from 'fs/promises';
|
||||
import { join } from 'path';
|
||||
|
||||
import { createJob, getJob, updateJob } from '$lib/server/db.js';
|
||||
import { retryJob } from '$lib/server/pipeline.js';
|
||||
import { getUploadPath } from '$lib/server/downloader.js';
|
||||
|
||||
describe('retryJob — missing file handling', () => {
|
||||
it('returns 200 immediately (does not throw) when upload file missing, sets job to failed async', async () => {
|
||||
const job = createJob('missing-test.webm', 'Missing File Test', 'auto');
|
||||
// Put the job in failed state (as if it failed originally)
|
||||
updateJob({ id: job.id, status: 'failed', error: 'original simulated failure' });
|
||||
|
||||
// retryJob should NOT throw — returns immediately (fire-and-forget IIFE)
|
||||
await expect(retryJob(job.id)).resolves.toBeUndefined();
|
||||
|
||||
// The IIFE runs asynchronously; wait for it to complete by polling the DB
|
||||
await vi.waitFor(
|
||||
() => {
|
||||
const updated = getJob(job.id)!;
|
||||
expect(updated.status).toBe('failed');
|
||||
expect(updated.error).toMatch(/^retry failed:/);
|
||||
},
|
||||
{ timeout: 3000, interval: 50 }
|
||||
);
|
||||
});
|
||||
|
||||
it('sets job to failed with descriptive ENOENT message when file is absent', async () => {
|
||||
const job = createJob('no-file-here.opus', 'No File', 'standard');
|
||||
updateJob({ id: job.id, status: 'failed', error: 'first failure' });
|
||||
|
||||
await expect(retryJob(job.id)).resolves.toBeUndefined();
|
||||
|
||||
await vi.waitFor(
|
||||
() => {
|
||||
const updated = getJob(job.id)!;
|
||||
expect(updated.status).toBe('failed');
|
||||
expect(updated.error).toMatch(/retry failed:.*ENOENT/i);
|
||||
},
|
||||
{ timeout: 3000, interval: 50 }
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves 404/409 path unchanged — throws on missing job', async () => {
|
||||
await expect(retryJob('00000000-0000-0000-0000-000000000000')).rejects.toThrow('Job not found');
|
||||
});
|
||||
});
|
||||
|
||||
describe('retryJob — file present case still triggers retry', () => {
|
||||
// getUploadPath resolves against default DATA_DIR (/tmp/.whisper-pwa)
|
||||
// because downloader.ts module-level DATA_DIR is evaluated at first import
|
||||
// and cached. Create the upload file there so readFile succeeds.
|
||||
const DEFAULT_UPLOAD_DIR = '/tmp/.whisper-pwa/uploads';
|
||||
|
||||
it('returns immediately and resets job when file exists (runJob fires async)', async () => {
|
||||
const job = createJob('present-test.mp3', 'File Present', 'auto');
|
||||
updateJob({ id: job.id, status: 'failed', error: 'simulated failure' });
|
||||
|
||||
// Create the upload file at the path getUploadPath returns
|
||||
const jobUploadDir = join(DEFAULT_UPLOAD_DIR, job.id);
|
||||
await mkdir(jobUploadDir, { recursive: true });
|
||||
const uploadPath = getUploadPath(job.id, 'present-test.mp3');
|
||||
await writeFile(uploadPath, Buffer.from('fake audio content'));
|
||||
|
||||
// retryJob must not throw — returns immediately
|
||||
await expect(retryJob(job.id)).resolves.toBeUndefined();
|
||||
|
||||
// resetJob() runs synchronously inside retryJob, so job status is
|
||||
// immediately 'pending' after retryJob returns (before IIFE continues)
|
||||
const updated = getJob(job.id)!;
|
||||
expect(updated.status).toBe('pending');
|
||||
expect(updated.error).toBeNull();
|
||||
|
||||
// Cleanup
|
||||
await rm(uploadPath, { force: true });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user