Challenge: If no retry-after header on 428, don't start timer for retry

ts/challenge.ts

@@ -14,7 +14,7 @@
 
 import { assert } from './util/assert';
 import { isOlderThan } from './util/timestamp';
-import { parseRetryAfter } from './util/parseRetryAfter';
+import { parseRetryAfterWithDefault } from './util/parseRetryAfter';
 import { clearTimeoutIfNecessary } from './util/clearTimeoutIfNecessary';
 import { getEnvironment, Environment } from './environment';
 import type { StorageInterface } from './types/Storage.d';
@@ -70,7 +70,7 @@ export const STORAGE_KEY = 'challenge:conversations';
 export type RegisteredChallengeType = Readonly<{
   conversationId: string;
   createdAt: number;
-  retryAt: number;
+  retryAt?: number;
   token?: string;
 }>;
 
@@ -80,7 +80,12 @@ const CAPTCHA_STAGING_URL =
   'https://signalcaptchas.org/staging/challenge/generate.html';
 
 function shouldStartQueue(registered: RegisteredChallengeType): boolean {
-  if (!registered.retryAt || registered.retryAt <= Date.now()) {
+  // No retryAt provided; waiting for user to complete captcha
+  if (!registered.retryAt) {
+    return false;
+  }
+
+  if (registered.retryAt <= Date.now()) {
     return true;
   }
 
@@ -214,21 +219,26 @@ export class ChallengeHandler {
       return;
     }
 
-    const waitTime = Math.max(0, challenge.retryAt - Date.now());
+    if (challenge.retryAt) {
-    const oldTimer = this.startTimers.get(conversationId);
+      const waitTime = Math.max(0, challenge.retryAt - Date.now());
-    if (oldTimer) {
+      const oldTimer = this.startTimers.get(conversationId);
-      clearTimeoutIfNecessary(oldTimer);
+      if (oldTimer) {
+        clearTimeoutIfNecessary(oldTimer);
+      }
+      this.startTimers.set(
+        conversationId,
+        setTimeout(() => {
+          this.startTimers.delete(conversationId);
+
+          this.startQueue(conversationId);
+        }, waitTime)
+      );
+      log.info(
+        `challenge: tracking ${conversationId} with waitTime=${waitTime}`
+      );
+    } else {
+      log.info(`challenge: tracking ${conversationId} with no waitTime`);
     }
-    this.startTimers.set(
-      conversationId,
-      setTimeout(() => {
-        this.startTimers.delete(conversationId);
-
-        this.startQueue(conversationId);
-      }, waitTime)
-    );
-
-    log.info(`challenge: tracking ${conversationId} with waitTime=${waitTime}`);
 
     if (data && !data.options?.includes('recaptcha')) {
       log.error(
@@ -379,7 +389,9 @@ export class ChallengeHandler {
         throw error;
       }
 
-      const retryAfter = parseRetryAfter(error.responseHeaders['retry-after']);
+      const retryAfter = parseRetryAfterWithDefault(
+        error.responseHeaders['retry-after']
+      );
 
       log.info(`challenge: retry after ${retryAfter}ms`);
       this.options.onChallengeFailed(retryAfter);

ts/jobs/helpers/findRetryAfterTimeFromError.ts

@@ -3,7 +3,7 @@
 
 import { isRecord } from '../../util/isRecord';
 import { HTTPError } from '../../textsecure/Errors';
-import { parseRetryAfter } from '../../util/parseRetryAfter';
+import { parseRetryAfterWithDefault } from '../../util/parseRetryAfter';
 
 export function findRetryAfterTimeFromError(err: unknown): number {
   let rawValue: unknown;
@@ -16,5 +16,5 @@ export function findRetryAfterTimeFromError(err: unknown): number {
     }
   }
 
-  return parseRetryAfter(rawValue);
+  return parseRetryAfterWithDefault(rawValue);
 }

ts/test-both/util/parseRetryAfter_test.ts

@@ -4,25 +4,25 @@
 import { assert } from 'chai';
 import { MINUTE } from '../../util/durations';
 
-import { parseRetryAfter } from '../../util/parseRetryAfter';
+import { parseRetryAfterWithDefault } from '../../util/parseRetryAfter';
 
 describe('parseRetryAfter', () => {
   it('should return 1 minute when passed non-strings', () => {
-    assert.equal(parseRetryAfter(undefined), MINUTE);
+    assert.equal(parseRetryAfterWithDefault(undefined), MINUTE);
-    assert.equal(parseRetryAfter(1234), MINUTE);
+    assert.equal(parseRetryAfterWithDefault(1234), MINUTE);
   });
 
   it('should return 1 minute with invalid strings', () => {
-    assert.equal(parseRetryAfter('nope'), MINUTE);
+    assert.equal(parseRetryAfterWithDefault('nope'), MINUTE);
-    assert.equal(parseRetryAfter('1ff'), MINUTE);
+    assert.equal(parseRetryAfterWithDefault('1ff'), MINUTE);
   });
 
   it('should return milliseconds on valid input', () => {
-    assert.equal(parseRetryAfter('100'), 100000);
+    assert.equal(parseRetryAfterWithDefault('100'), 100000);
   });
 
   it('should return 1 second at minimum', () => {
-    assert.equal(parseRetryAfter('0'), 1000);
+    assert.equal(parseRetryAfterWithDefault('0'), 1000);
-    assert.equal(parseRetryAfter('-1'), 1000);
+    assert.equal(parseRetryAfterWithDefault('-1'), 1000);
   });
 });

ts/textsecure/Errors.ts

@@ -156,7 +156,7 @@ export class SendMessageChallengeError extends ReplayableError {
 
   public readonly data: SendMessageChallengeData | undefined;
 
-  public readonly retryAt: number;
+  public readonly retryAt?: number;
 
   constructor(identifier: string, httpError: HTTPError) {
     super({
@@ -171,7 +171,10 @@ export class SendMessageChallengeError extends ReplayableError {
 
     const headers = httpError.responseHeaders || {};
 
-    this.retryAt = Date.now() + parseRetryAfter(headers['retry-after']);
+    const retryAfter = parseRetryAfter(headers['retry-after']);
+    if (retryAfter) {
+      this.retryAt = Date.now() + retryAfter;
+    }
 
     appendStack(this, httpError);
   }

ts/util/parseRetryAfter.ts

@@ -7,15 +7,24 @@ import { isNormalNumber } from './isNormalNumber';
 const DEFAULT_RETRY_AFTER = MINUTE;
 const MINIMAL_RETRY_AFTER = SECOND;
 
-export function parseRetryAfter(value: unknown): number {
+export function parseRetryAfterWithDefault(value: unknown): number {
-  if (typeof value !== 'string') {
+  const retryAfter = parseRetryAfter(value);
+  if (retryAfter === undefined) {
     return DEFAULT_RETRY_AFTER;
   }
 
+  return Math.max(retryAfter, MINIMAL_RETRY_AFTER);
+}
+
+export function parseRetryAfter(value: unknown): number | undefined {
+  if (typeof value !== 'string') {
+    return undefined;
+  }
+
   const retryAfter = parseInt(value, 10);
   if (!isNormalNumber(retryAfter) || retryAfter.toString() !== value) {
-    return DEFAULT_RETRY_AFTER;
+    return undefined;
   }
 
-  return Math.max(retryAfter * SECOND, MINIMAL_RETRY_AFTER);
+  return retryAfter * SECOND;
 }