Commit 6f1d62f
fix(backoff): allow no delay arg when setting immediate strategy (#154)
Per [the
documentation](https://github.com/mixmaxhq/bee-queue#jobbackoffstrategy-delayfactor)
you should be able to call `job.backoff('immediate')` to set an
immediate backoff strategy for retried jobs. However, the `backoff`
method explicitly disallowed this by checking for
`Number.isSafeInteger(undefined)` which returns false. This means
you currently cannot specify an immediate backoff strategy without also specifying a positive delay value.
To fix this, only check for valid integers when the strategy is not
`immediate`.
Co-authored-by: Hugh Secker-Walker <hsw@hodain.net>1 parent b9ffac9 commit 6f1d62f
3 files changed
Lines changed: 38 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -85,7 +85,7 @@ declare namespace BeeQueue { | |||
| 85 | 85 | ||
| 86 | 86 | setId(id: string): this; | |
| 87 | 87 | retries(n: number): this; | |
| 88 | - backoff(strategy: "immediate" | "fixed" | "exponential", delayFactor: number): this; | ||
| 88 | + backoff(strategy: "immediate" | "fixed" | "exponential", delayFactor?: number): this; | ||
| 89 | 89 | delayUntil(dateOrTimestamp: Date | number): this; | |
| 90 | 90 | timeout(milliseconds: number): this; | |
| 91 | 91 | save(): Promise<this>; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -124,7 +124,9 @@ class Job extends Emitter { | |||
| 124 | 124 | if (!strategies.has(strategy)) { | |
| 125 | 125 | throw new Error('unknown strategy'); | |
| 126 | 126 | } | |
| 127 | - if (!Number.isSafeInteger(delay) || delay <= 0) { | ||
| 127 | + | ||
| 128 | + const isInvalidDelay = !Number.isSafeInteger(delay) || delay <= 0; | ||
| 129 | + if (strategy !== 'immediate' && isInvalidDelay) { | ||
| 128 | 130 | throw new Error('delay must be a positive integer'); | |
| 129 | 131 | } | |
| 130 | 132 | this.options.backoff = { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1464,6 +1464,40 @@ describe('Queue', (it) => { | |||
| 1464 | 1464 | t.true(calls[1] - calls[0] >= 100); | |
| 1465 | 1465 | }); | |
| 1466 | 1466 | ||
| 1467 | + it('should handle immediate backoff', async (t) => { | ||
| 1468 | + const queue = t.context.makeQueue({ | ||
| 1469 | + activateDelayedJobs: true | ||
| 1470 | + }); | ||
| 1471 | + | ||
| 1472 | + const calls = []; | ||
| 1473 | + | ||
| 1474 | + queue.process(async (job) => { | ||
| 1475 | + t.deepEqual(job.options.backoff, { | ||
| 1476 | + strategy: 'immediate' | ||
| 1477 | + }); | ||
| 1478 | + t.deepEqual(job.data, {is: 'immediate'}); | ||
| 1479 | + calls.push(Date.now()); | ||
| 1480 | + if (calls.length === 1) { | ||
| 1481 | + throw new Error('forced retry'); | ||
| 1482 | + } | ||
| 1483 | + t.is(calls.length, 2); | ||
| 1484 | + }); | ||
| 1485 | + | ||
| 1486 | + const succeed = helpers.waitOn(queue, 'succeeded', true); | ||
| 1487 | + | ||
| 1488 | + await queue.createJob({is: 'immediate'}) | ||
| 1489 | + .retries(2) | ||
| 1490 | + .backoff('immediate') | ||
| 1491 | + .save(); | ||
| 1492 | + | ||
| 1493 | + await succeed; | ||
| 1494 | + | ||
| 1495 | + t.is(calls.length, 2); | ||
| 1496 | + | ||
| 1497 | + // Temporal sanity | ||
| 1498 | + t.true(calls[1] >= calls[0]); | ||
| 1499 | + }); | ||
| 1500 | + | ||
| 1467 | 1501 | it('should handle exponential backoff', async (t) => { | |
| 1468 | 1502 | const queue = t.context.makeQueue({ | |
| 1469 | 1503 | activateDelayedJobs: true | |
0 commit comments