Skip to content

Commit 0840588

Browse files
princjefsimison
authored andcommitted
Fix: make touch promise-based (#667)
The [`job.touch()`](https://github.com/agenda/agenda/blob/ff94c8a4c9bc564a0bed9eaa79de1c4fdbed0fde/lib/job/touch.js#L10-L13) method didn't get migrated over to promises along with everything else and still expects a callback. However, when it passes that callback down to `job.save()`, an error is thrown because a callback is not allowed. This managed to slip through because the test for it changed at some point to mock out the implementation of `job.save()` with a callback version that didn't throw when it was called with a callback. https://github.com/agenda/agenda/blob/bd8a8e003cd09d6e9826accbf6c30be75212a9a9/test/job.js#L352-L364 This PR makes the behavior of `touch()` align with the rest of the library and updates the associated test to remove the mock on `save()`.
1 parent ff94c8a commit 0840588

File tree

3 files changed

+20
-24
lines changed

3 files changed

+20
-24
lines changed

README.md

+10-11
Original file line numberDiff line numberDiff line change
@@ -739,22 +739,21 @@ Disables the `job`. Upcoming runs won't execute.
739739

740740
Enables the `job` if it got disabled before. Upcoming runs will execute.
741741

742-
### touch(callback)
742+
### touch()
743743

744744
Resets the lock on the job. Useful to indicate that the job hasn't timed out
745-
when you have very long running jobs.
745+
when you have very long running jobs. The call returns a promise that resolves
746+
when the job's lock has been renewed.
746747

747748
```js
748749
agenda.define('super long job', (job, done) => {
749-
doSomeLongTask(() => {
750-
job.touch(() => {
751-
doAnotherLongTask(() => {
752-
job.touch(() => {
753-
finishOurLongTasks(done);
754-
});
755-
});
756-
});
757-
});
750+
(async () => {
751+
await doSomeLongTask();
752+
await job.touch();
753+
await doAnotherLongTask();
754+
await job.touch();
755+
await finishOurLongTasks();
756+
})().then(done, done);
758757
});
759758
```
760759

lib/job/touch.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
'use strict';
22

3+
const noCallback = require('../no-callback');
4+
35
/**
46
* Updates "lockedAt" time so the job does not get picked up again
57
* @name Job#touch
68
* @function
7-
* @param {Function} cb called when job "touch" fails or passes
89
* @returns {undefined}
910
*/
10-
module.exports = function(cb) {
11+
module.exports = async function() {
12+
// eslint-disable-next-line prefer-rest-params
13+
noCallback(arguments);
1114
this.attrs.lockedAt = new Date();
12-
this.save(cb);
15+
return this.save();
1316
};

test/job.js

+4-10
Original file line numberDiff line numberDiff line change
@@ -349,18 +349,12 @@ describe('Job', () => {
349349
});
350350

351351
describe('touch', () => {
352-
it('extends the lock lifetime', done => {
352+
it('extends the lock lifetime', async() => {
353353
const lockedAt = new Date();
354354
const job = new Job({agenda, name: 'some job', lockedAt});
355-
job.save = function(cb) {
356-
cb();
357-
};
358-
setTimeout(() => {
359-
job.touch(() => {
360-
expect(job.attrs.lockedAt).to.be.greaterThan(lockedAt);
361-
done();
362-
});
363-
}, 2);
355+
await delay(2);
356+
await job.touch();
357+
expect(job.attrs.lockedAt).to.be.greaterThan(lockedAt);
364358
});
365359
});
366360

0 commit comments

Comments
 (0)