fix: isolate per-job cleanup errors and guard markDone against undefined
runCleanup previously had no try/catch around each expired job, so one job's markCleaned/delete failure would abort the whole pass, leaving later expired jobs' files undeleted for that run. Each iteration is now wrapped in a try/catch that logs and continues; the returned count only reflects jobs that actually completed the delete+markCleaned sequence. markDone now guards outputPath/outputMimeType/outputSizeBytes/ conversionDurationSeconds with `?? null`, matching the guard createJob already has on `quality` — Prisma treats `undefined` in a data object as "leave the column alone" rather than binding NULL like the old raw SQL did. Currently unreachable in practice since the worker always passes real values, but keeps the repository defensive and consistent. Added a cleanup.test.js case that monkey-patches prisma.conversionJob.update to reject for one job's cleanedAt update, asserting a later expired job in the same batch still gets cleaned. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+7
-1
@@ -7,13 +7,19 @@ import { findExpiredJobs, markCleaned } from './jobs/jobRepository.js';
|
|||||||
export async function runCleanup(prisma, config) {
|
export async function runCleanup(prisma, config) {
|
||||||
const expiredJobs = await findExpiredJobs(prisma);
|
const expiredJobs = await findExpiredJobs(prisma);
|
||||||
|
|
||||||
|
let cleanedCount = 0;
|
||||||
for (const job of expiredJobs) {
|
for (const job of expiredJobs) {
|
||||||
|
try {
|
||||||
await deleteIfExists(uploadPath(config, job.uuid, job.sourceFormat));
|
await deleteIfExists(uploadPath(config, job.uuid, job.sourceFormat));
|
||||||
await deleteIfExists(outputPath(config, job.uuid, job.targetFormat));
|
await deleteIfExists(outputPath(config, job.uuid, job.targetFormat));
|
||||||
await markCleaned(prisma, job.id);
|
await markCleaned(prisma, job.id);
|
||||||
|
cleanedCount += 1;
|
||||||
|
} catch (err) {
|
||||||
|
console.error(`Cleanup failed for job ${job.uuid}:`, err);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return expiredJobs.length;
|
return cleanedCount;
|
||||||
}
|
}
|
||||||
|
|
||||||
async function main() {
|
async function main() {
|
||||||
|
|||||||
@@ -54,7 +54,13 @@ export async function markProcessing(prisma, id) {
|
|||||||
export async function markDone(prisma, id, { outputPath, outputMimeType, outputSizeBytes, conversionDurationSeconds }) {
|
export async function markDone(prisma, id, { outputPath, outputMimeType, outputSizeBytes, conversionDurationSeconds }) {
|
||||||
await prisma.conversionJob.update({
|
await prisma.conversionJob.update({
|
||||||
where: { id },
|
where: { id },
|
||||||
data: { status: 'done', outputPath, outputMimeType, outputSizeBytes, conversionDurationSeconds },
|
data: {
|
||||||
|
status: 'done',
|
||||||
|
outputPath: outputPath ?? null,
|
||||||
|
outputMimeType: outputMimeType ?? null,
|
||||||
|
outputSizeBytes: outputSizeBytes ?? null,
|
||||||
|
conversionDurationSeconds: conversionDurationSeconds ?? null,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -105,6 +105,55 @@ describe('runCleanup', () => {
|
|||||||
expect(job.cleanedAt).toBeNull();
|
expect(job.cleanedAt).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('isolates one job’s failure so later expired jobs in the same batch still get cleaned', async () => {
|
||||||
|
const failingUuid = '14141414-1414-4414-8414-141414141414';
|
||||||
|
const okUuid = '15151515-1515-4515-8515-151515151515';
|
||||||
|
await createJob(prisma, {
|
||||||
|
uuid: failingUuid,
|
||||||
|
family: 'image',
|
||||||
|
sourceFormat: 'png',
|
||||||
|
targetFormat: 'webp',
|
||||||
|
originalFilename: 'photo.png',
|
||||||
|
inputPath: `${failingUuid}.png`,
|
||||||
|
inputMimeType: 'image/png',
|
||||||
|
inputSizeBytes: 11,
|
||||||
|
expiresAt: new Date(Date.now() - 1000),
|
||||||
|
});
|
||||||
|
await createJob(prisma, {
|
||||||
|
uuid: okUuid,
|
||||||
|
family: 'image',
|
||||||
|
sourceFormat: 'png',
|
||||||
|
targetFormat: 'webp',
|
||||||
|
originalFilename: 'photo.png',
|
||||||
|
inputPath: `${okUuid}.png`,
|
||||||
|
inputMimeType: 'image/png',
|
||||||
|
inputSizeBytes: 11,
|
||||||
|
expiresAt: new Date(Date.now() - 1000),
|
||||||
|
});
|
||||||
|
const failingJob = await getJobByUuid(prisma, failingUuid);
|
||||||
|
|
||||||
|
const originalUpdate = prisma.conversionJob.update.bind(prisma.conversionJob);
|
||||||
|
prisma.conversionJob.update = (args) => {
|
||||||
|
if (args.where?.id === failingJob.id) {
|
||||||
|
return Promise.reject(new Error('simulated markCleaned failure'));
|
||||||
|
}
|
||||||
|
return originalUpdate(args);
|
||||||
|
};
|
||||||
|
|
||||||
|
let cleanedCount;
|
||||||
|
try {
|
||||||
|
cleanedCount = await runCleanup(prisma, config);
|
||||||
|
} finally {
|
||||||
|
prisma.conversionJob.update = originalUpdate;
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(cleanedCount).toBe(1);
|
||||||
|
const failing = await getJobByUuid(prisma, failingUuid);
|
||||||
|
expect(failing.cleanedAt).toBeNull();
|
||||||
|
const ok = await getJobByUuid(prisma, okUuid);
|
||||||
|
expect(ok.cleanedAt).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
it('does not re-process an already cleaned expired job', async () => {
|
it('does not re-process an already cleaned expired job', async () => {
|
||||||
const uuid = '13131313-1313-4313-8313-131313131313';
|
const uuid = '13131313-1313-4313-8313-131313131313';
|
||||||
await createJob(prisma, {
|
await createJob(prisma, {
|
||||||
|
|||||||
Reference in New Issue
Block a user