diff --git a/src/cli.ts b/src/cli.ts index a60575f..f629407 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -112,7 +112,7 @@ const cli = meow( directoryListing: {type: 'boolean'}, retry: {type: 'boolean'}, retryErrors: {type: 'boolean'}, - retryErrorsCount: {type: 'number', default: 3}, + retryErrorsCount: {type: 'number', default: 5}, retryErrorsJitter: {type: 'number', default: 3000}, urlRewriteSearch: {type: 'string'}, urlReWriteReplace: {type: 'string'}, diff --git a/src/index.ts b/src/index.ts index 7f19c6c..163506e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -135,7 +135,7 @@ export class LinkChecker extends EventEmitter { rootPath: path, retry: !!opts.retry, retryErrors: !!opts.retryErrors, - retryErrorsCount: opts.retryErrorsCount ?? 3, + retryErrorsCount: opts.retryErrorsCount ?? 5, retryErrorsJitter: opts.retryErrorsJitter ?? 3000, }); }); @@ -446,7 +446,6 @@ export class LinkChecker extends EventEmitter { */ shouldRetryOnError(status: number, opts: CrawlOptions): boolean { const maxRetries = opts.retryErrorsCount; - const retryAfter = opts.retryErrorsJitter; if (!opts.retryErrors) { return false; @@ -458,14 +457,19 @@ export class LinkChecker extends EventEmitter { } // check to see if there is already a request to wait for this host + let currentRetries = 1; if (opts.retryErrorsCache.has(opts.url.host)) { // use whichever time is higher in the cache - const currentRetries = opts.retryErrorsCache.get(opts.url.host)!; + currentRetries = opts.retryErrorsCache.get(opts.url.host)!; if (currentRetries > maxRetries) return false; opts.retryErrorsCache.set(opts.url.host, currentRetries + 1); } else { opts.retryErrorsCache.set(opts.url.host, 1); } + // Use exponential backoff algorithm to take pressure off upstream service: + const retryAfter = + Math.pow(2, currentRetries) * 1000 + + Math.random() * opts.retryErrorsJitter; opts.queue.add( async () => { diff --git a/src/queue.ts b/src/queue.ts index b813d65..3ea6553 100644 --- a/src/queue.ts +++ b/src/queue.ts @@ -53,16 +53,23 @@ export class Queue extends EventEmitter { } // grab the element at the front of the array const item = this.q.shift()!; + // Depending on CPU load and other factors setTimeout() is not guranteed to run exactly + // when scheduled. This causes problems if there is only one item in the queue, as + // there's a chance it will never be processed. Allow for a small delta to address this: + const delta = 150; + const readyToExecute = + Math.abs(item.timeToRun - Date.now()) < delta || + item.timeToRun < Date.now(); // make sure this element is ready to execute - if not, to the back of the stack - if (item.timeToRun > Date.now()) { - this.q.push(item); - } else { + if (readyToExecute) { // this function is ready to go! this.activeFunctions++; item.fn().finally(() => { this.activeFunctions--; this.tick(); }); + } else { + this.q.push(item); } } } diff --git a/test/test.retry.ts b/test/test.retry.ts index 347dfa5..1ab492b 100644 --- a/test/test.retry.ts +++ b/test/test.retry.ts @@ -257,5 +257,28 @@ describe('retries', () => { assert.ok(results.passed); scope.done(); }); + + it('should eventually stop retrying', async () => { + const scope = nock('http://fake.local') + .get('/') + .replyWithError({code: 'ETIMEDOUT'}); + + const {promise, resolve} = invertedPromise(); + const checker = new LinkChecker().on('retry', resolve); + const clock = sinon.useFakeTimers({ + shouldAdvanceTime: true, + }); + const checkPromise = checker.check({ + path: 'test/fixtures/basic', + retryErrors: true, + retryErrorsCount: 1, + retryErrorsJitter: 1000, + }); + await promise; + await clock.tickAsync(5000); + const results = await checkPromise; + assert.ok(!results.passed); + scope.done(); + }); }); });