From 84dc49f2ea617978247dc6516091fc48fd8163f3 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Mon, 31 Jan 2022 13:31:16 -0800 Subject: [PATCH] Increase fallback `Retry-After` time to 1 minute Co-authored-by: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> --- ts/test-both/util/parseRetryAfter_test.ts | 15 ++++++++------- .../helpers/findRetryAfterTimeFromError_test.ts | 13 ++++++++----- .../helpers/sleepFor413RetryAfterTime_test.ts | 8 ++++---- ts/util/parseRetryAfter.ts | 9 +++++---- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/ts/test-both/util/parseRetryAfter_test.ts b/ts/test-both/util/parseRetryAfter_test.ts index 11bd59355..e0642f0d3 100644 --- a/ts/test-both/util/parseRetryAfter_test.ts +++ b/ts/test-both/util/parseRetryAfter_test.ts @@ -1,19 +1,20 @@ -// Copyright 2021 Signal Messenger, LLC +// Copyright 2021-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only import { assert } from 'chai'; +import { MINUTE } from '../../util/durations'; import { parseRetryAfter } from '../../util/parseRetryAfter'; describe('parseRetryAfter', () => { - it('should return 1 second when passed non-strings', () => { - assert.equal(parseRetryAfter(undefined), 1000); - assert.equal(parseRetryAfter(1234), 1000); + it('should return 1 minute when passed non-strings', () => { + assert.equal(parseRetryAfter(undefined), MINUTE); + assert.equal(parseRetryAfter(1234), MINUTE); }); - it('should return 1 second with invalid strings', () => { - assert.equal(parseRetryAfter('nope'), 1000); - assert.equal(parseRetryAfter('1ff'), 1000); + it('should return 1 minute with invalid strings', () => { + assert.equal(parseRetryAfter('nope'), MINUTE); + assert.equal(parseRetryAfter('1ff'), MINUTE); }); it('should return milliseconds on valid input', () => { diff --git a/ts/test-node/jobs/helpers/findRetryAfterTimeFromError_test.ts b/ts/test-node/jobs/helpers/findRetryAfterTimeFromError_test.ts index a5f808eae..454f39e05 100644 --- a/ts/test-node/jobs/helpers/findRetryAfterTimeFromError_test.ts +++ b/ts/test-node/jobs/helpers/findRetryAfterTimeFromError_test.ts @@ -5,15 +5,18 @@ import { assert } from 'chai'; import { findRetryAfterTimeFromError } from '../../../jobs/helpers/findRetryAfterTimeFromError'; import { HTTPError } from '../../../textsecure/Errors'; +import { MINUTE } from '../../../util/durations'; describe('findRetryAfterTimeFromError', () => { - it('returns 1 second if no Retry-After time is found', () => { + it('returns 1 minute if no Retry-After time is found', () => { [ undefined, null, {}, { responseHeaders: {} }, { responseHeaders: { 'retry-after': 'garbage' } }, + { responseHeaders: { 'retry-after': '0.5' } }, + { responseHeaders: { 'retry-after': '12.34' } }, { httpError: new HTTPError('Slow down', { code: 413, @@ -29,20 +32,20 @@ describe('findRetryAfterTimeFromError', () => { }), }, ].forEach(input => { - assert.strictEqual(findRetryAfterTimeFromError(input), 1000); + assert.strictEqual(findRetryAfterTimeFromError(input), MINUTE); }); }); it("returns 1 second if a Retry-After time is found, but it's less than 1 second", () => { - ['0', '-99', '0.5'].forEach(headerValue => { + ['0', '-99'].forEach(headerValue => { const input = { responseHeaders: { 'retry-after': headerValue } }; assert.strictEqual(findRetryAfterTimeFromError(input), 1000); }); }); - it('returns 1 second for extremely large numbers', () => { + it('returns 1 minute for extremely large numbers', () => { const input = { responseHeaders: { 'retry-after': '999999999999999999' } }; - assert.strictEqual(findRetryAfterTimeFromError(input), 1000); + assert.strictEqual(findRetryAfterTimeFromError(input), MINUTE); }); it('finds the retry-after time on top-level response headers', () => { diff --git a/ts/test-node/jobs/helpers/sleepFor413RetryAfterTime_test.ts b/ts/test-node/jobs/helpers/sleepFor413RetryAfterTime_test.ts index cc03e86d5..763bc52a7 100644 --- a/ts/test-node/jobs/helpers/sleepFor413RetryAfterTime_test.ts +++ b/ts/test-node/jobs/helpers/sleepFor413RetryAfterTime_test.ts @@ -1,4 +1,4 @@ -// Copyright 2021 Signal Messenger, LLC +// Copyright 2021-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only import { assert } from 'chai'; @@ -39,19 +39,19 @@ describe('sleepFor413RetryAfterTimeIfApplicable', () => { sinon.assert.notCalled(log.info); }); - it('waits for 1 second if the error lacks Retry-After info', async () => { + it('waits for 1 minute if the error lacks Retry-After info', async () => { let done = false; (async () => { await sleepFor413RetryAfterTime({ err: {}, log: createLogger(), - timeRemaining: 1234, + timeRemaining: 12345678, }); done = true; })(); - await clock.tickAsync(999); + await clock.tickAsync(durations.MINUTE - 1); assert.isFalse(done); await clock.tickAsync(2); diff --git a/ts/util/parseRetryAfter.ts b/ts/util/parseRetryAfter.ts index ecbcee562..546e26381 100644 --- a/ts/util/parseRetryAfter.ts +++ b/ts/util/parseRetryAfter.ts @@ -1,19 +1,20 @@ // Copyright 2021-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import { SECOND } from './durations'; +import { SECOND, MINUTE } from './durations'; import { isNormalNumber } from './isNormalNumber'; +const DEFAULT_RETRY_AFTER = MINUTE; const MINIMAL_RETRY_AFTER = SECOND; export function parseRetryAfter(value: unknown): number { if (typeof value !== 'string') { - return MINIMAL_RETRY_AFTER; + return DEFAULT_RETRY_AFTER; } - let retryAfter = parseInt(value, 10); + const retryAfter = parseInt(value, 10); if (!isNormalNumber(retryAfter) || retryAfter.toString() !== value) { - retryAfter = 0; + return DEFAULT_RETRY_AFTER; } return Math.max(retryAfter * SECOND, MINIMAL_RETRY_AFTER);