Skip to content

Commit 606e141

Browse files
committed
feat: check if job state update was successful before running a job
this stops processing jobs when they got removed in the meantime from the database
1 parent e6b038c commit 606e141

File tree

3 files changed

+55
-3
lines changed

3 files changed

+55
-3
lines changed

src/Job.ts

+8-2
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,14 @@ export class Job<DATA = unknown | void> {
377377
log('[%s:%s] has failed [%s]', this.attrs.name, this.attrs._id, error.message);
378378
} finally {
379379
this.attrs.lockedAt = undefined;
380-
await this.agenda.db.saveJobState(this);
381-
log('[%s:%s] was saved successfully to MongoDB', this.attrs.name, this.attrs._id);
380+
try {
381+
await this.agenda.db.saveJobState(this);
382+
log('[%s:%s] was saved successfully to MongoDB', this.attrs.name, this.attrs._id);
383+
} catch (err) {
384+
// in case this fails, we ignore it
385+
// this can e.g. happen if the job gets removed during the execution
386+
log('[%s:%s] was not saved to MongoDB', this.attrs.name, this.attrs._id, err);
387+
}
382388

383389
this.agenda.emit('complete', this);
384390
this.agenda.emit(`complete:${this.attrs.name}`, this);

src/JobDbRepository.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,18 @@ export class JobDbRepository {
256256

257257
log('[job %s] save job state: \n%O', id, $set);
258258

259-
await this.collection.updateOne(
259+
const result = await this.collection.updateOne(
260260
{ _id: id, name: job.attrs.name },
261261
{
262262
$set
263263
}
264264
);
265+
266+
if (!result.acknowledged || result.matchedCount !== 1) {
267+
throw new Error(
268+
`job ${id} (name: ${job.attrs.name}) cannot be updated in the database, maybe it does not exist anymore?`
269+
);
270+
}
265271
}
266272

267273
/**

test/job.test.ts

+40
Original file line numberDiff line numberDiff line change
@@ -1601,4 +1601,44 @@ describe('Job', () => {
16011601

16021602
expect(await job.isRunning()).to.be.equal(true);
16031603
});
1604+
1605+
it('should not run job if is has been removed', async () => {
1606+
let executed = false;
1607+
agenda.define('test', async () => {
1608+
executed = true;
1609+
});
1610+
1611+
const job = new Job(agenda, {
1612+
name: 'test',
1613+
type: 'normal'
1614+
});
1615+
job.schedule('in 1 second');
1616+
await job.save();
1617+
1618+
await agenda.start();
1619+
1620+
// wait till it's locked (Picked up by the event processor)
1621+
const jobStarted = await agenda.db.getJobs({ name: 'test' });
1622+
expect(jobStarted[0].lockedAt).to.not.equal(null);
1623+
1624+
await job.remove();
1625+
1626+
let error;
1627+
const completed = new Promise<void>(resolve => {
1628+
agenda.on('error', err => {
1629+
error = err;
1630+
resolve();
1631+
});
1632+
});
1633+
1634+
await Promise.race([
1635+
new Promise(resolve => {
1636+
setTimeout(resolve, 1000);
1637+
}),
1638+
completed
1639+
]);
1640+
1641+
expect(executed).to.be.equal(false);
1642+
expect(error?.message).to.includes('(name: test) cannot be updated in the database');
1643+
});
16041644
});

0 commit comments

Comments
 (0)