fix: stop bandwidth measurements on terminal HTTP errors

src/engines/BandwidthEngine/BandwidthEngine.ts

@@ -4,7 +4,27 @@ import {
   type AuthorizationOptions
 } from '../../utils/authorization';
 
-const MAX_RETRIES = 20;
+const MAX_RETRIES = 3;
+
+class HttpError extends Error {
+  constructor(
+    readonly status: number,
+    statusText: string,
+    readonly retryAfter: string | null
+  ) {
+    super(statusText);
+  }
+}
+
+const getRetryDelay = (retryAfter: string | null): number => {
+  if (!retryAfter) return 5000;
+
+  const seconds = Number(retryAfter);
+  if (Number.isFinite(seconds) && seconds >= 0) return seconds * 1000;
+
+  const date = Date.parse(retryAfter);
+  return Number.isNaN(date) ? 5000 : Math.max(0, date - Date.now());
+};
 
 const ESTIMATED_HEADER_FRACTION = 0.005; // ~.5% of packet header / payload size. used when transferSize is not available.
 
@@ -230,19 +250,23 @@ class BandwidthMeasurementEngine implements Engine {
   set onFinished(f: (results: BandwidthEngineResults) => void) {
     this.#onFinished = f;
   }
-  #onConnectionError: (error: string) => void = () => {}; // Invoked when unable to get a response from the API
-  set onConnectionError(f: (error: string) => void) {
+  #onConnectionError: (error: string, status?: number) => void = () => {}; // Invoked when unable to get a response from the API
+  set onConnectionError(f: (error: string, status?: number) => void) {
     this.#onConnectionError = f;
   }
 
   // Public methods
   pause(): void {
+    if (this.#retryTimeout !== undefined) {
+      clearTimeout(this.#retryTimeout);
+      this.#retryTimeout = undefined;
+    }
     this.#cancelCurrentMeasurement(`pause()`);
     this.#setRunning(false);
   }
 
   play(): void {
-    if (!this.#running) {
+    if (!this.#failed && !this.#running) {
       this.#setRunning(true);
       this.#nextMeasurement();
     }
@@ -254,11 +278,13 @@ class BandwidthMeasurementEngine implements Engine {
   #uploadApi: string;
 
   #running: boolean = false;
+  #failed: boolean = false;
   #finished: Record<string, boolean> = { down: false, up: false };
   #results: BandwidthEngineResults = { down: {}, up: {} };
   #measIdx: number = 0;
   #counter: number = 0;
   #retries: number = 0;
+  #retryTimeout: ReturnType<typeof setTimeout> | undefined;
   #minDuration: number = -Infinity; // of current measurement
   #throttleMs: number = 0;
   #estimatedServerTime: number = 0;
@@ -397,7 +423,11 @@ class BandwidthMeasurementEngine implements Engine {
       url
     );
 
-    if (this.#retries === 0) {
+    if (
+      this.#retries === 0 ||
+      !this.#currentAbortController ||
+      this.#currentAbortController.signal.aborted
+    ) {
       // abort existing abort controller
       this.#currentAbortController?.abort('restarting engine');
 
@@ -406,9 +436,9 @@ class BandwidthMeasurementEngine implements Engine {
       if (this.abortRequestDuration) {
         const abortTimeout = setTimeout(() => {
           const errorMessage = `${isDown ? 'Download' : 'Upload'} measurement of ${numBytes} bytes aborted. Measurement exceeded bandwidthAbortRequestDuration (${this.abortRequestDuration}ms)`;
-          this.#cancelCurrentMeasurement(errorMessage);
           this.#retries = 0;
-          this.#setRunning(false);
+          this.pause();
+          this.#failed = true;
           this.#onConnectionError(errorMessage);
         }, this.abortRequestDuration);
         this.#currentAbortController.signal.addEventListener('abort', () =>
@@ -424,7 +454,11 @@ class BandwidthMeasurementEngine implements Engine {
     })
       .then(r => {
         if (r.ok) return r;
-        throw Error(r.statusText);
+        throw new HttpError(
+          r.status,
+          r.statusText,
+          r.headers.get('retry-after')
+        );
       })
       .then(r => {
         this.getServerTime && (serverTime = this.getServerTime(r));
@@ -567,15 +601,29 @@ class BandwidthMeasurementEngine implements Engine {
         }
         console.warn(`Error fetching ${url}: ${error}`);
 
-        if (this.#retries++ < MAX_RETRIES) {
-          this.#nextMeasurement(); // keep trying
-        } else {
-          this.#retries = 0;
-          this.#setRunning(false);
-          this.#onConnectionError(
-            `Connection failed to ${url}. Gave up after ${MAX_RETRIES} retries.`
-          );
+        if (
+          error instanceof HttpError &&
+          error.status === 429 &&
+          this.#retries++ < MAX_RETRIES
+        ) {
+          this.#retryTimeout = setTimeout(() => {
+            this.#retryTimeout = undefined;
+            if (this.#running && !this.#failed) this.#nextMeasurement();
+          }, getRetryDelay(error.retryAfter));
+          return;
         }
+
+        const status = error instanceof HttpError ? error.status : undefined;
+        const message =
+          status === 429
+            ? `Connection failed to ${url}. Gave up after ${MAX_RETRIES} retries.`
+            : status === undefined
+              ? `Connection failed to ${url}.`
+              : `Request failed with ${status}: ${url}`;
+        this.#retries = 0;
+        this.pause();
+        this.#failed = true;
+        this.#onConnectionError(message, status);
       });
   }
 

src/engines/BandwidthEngine/ParallelLatency.ts

@@ -96,11 +96,19 @@ class BandwidthWithParallelLatencyEngine extends BandwidthEngine {
     };
   }
 
-  set onConnectionError(onConnectionError: (error: string) => void) {
-    super.onConnectionError = (...args: [string]) => {
+  set onConnectionError(
+    onConnectionError: (error: string, status?: number) => void
+  ) {
+    super.onConnectionError = (...args: [string, number?]) => {
       this.#latencyEngine && this.#latencyEngine.pause();
       onConnectionError(...args);
     };
+    if (this.#latencyEngine) {
+      this.#latencyEngine.onConnectionError = (...args: [string, number?]) => {
+        this.pause();
+        onConnectionError(...args);
+      };
+    }
   }
 
   // Internal state

src/index.ts

@@ -175,9 +175,9 @@ class MeasurementEngine {
     this.#onFinish = f;
   }
 
-  #onError: (message: string) => void = () => {};
+  #onError: (message: string, status?: number) => void = () => {};
 
-  set onError(f: (message: string) => void) {
+  set onError(f: (message: string, status?: number) => void) {
     this.#onError = f;
   }
 
@@ -201,7 +201,7 @@ class MeasurementEngine {
    * resumes the current phase.
    */
   play(): void {
-    if (!this.#running) {
+    if (!this.#failed && !this.#running) {
       // Clear timings before running the engine
       performance.clearResourceTimings();
 
@@ -244,6 +244,7 @@ class MeasurementEngine {
 
   #running: boolean = false;
   #finished: boolean = false;
+  #failed: boolean = false;
 
   // Internal methods
   #setRunning(running: boolean): void {
@@ -297,6 +298,7 @@ class MeasurementEngine {
 
     this.#setRunning(false);
     this.#setFinished(false);
+    this.#failed = false;
 
     this.#results.clear();
     this.#accumulatedRuntimeMs = 0;
@@ -561,12 +563,12 @@ class MeasurementEngine {
           this.#running && this.#next();
         };
 
-        engine.onConnectionError = (e: unknown) => {
-          this.#serverTimeDelta = (engine as BandwidthEngine).serverTimeDelta;
+        engine.onConnectionError = (e: unknown, status?: number) => {
           msmResults.error = e;
+          this.#failed = true;
+          this.pause();
           this.onResultsChange({ type });
-          this.#onError(`Connection error while measuring latency: ${e}`);
-          this.#next();
+          this.#onError(String(e), status);
         };
 
         (engine as Engine & { play: () => void }).play!();
@@ -716,12 +718,12 @@ class MeasurementEngine {
             this.#running && this.#next();
           };
 
-          engine.onConnectionError = (e: unknown) => {
-            this.#serverTimeDelta = (engine as BandwidthEngine).serverTimeDelta;
+          engine.onConnectionError = (e: unknown, status?: number) => {
             msmResults.error = e;
+            this.#failed = true;
+            this.pause();
             this.onResultsChange({ type });
-            this.#onError(`Connection error while measuring ${type}: ${e}`);
-            this.#next();
+            this.#onError(String(e), status);
           };
 
           (engine as Engine & { play: () => void }).play!();