fix: continue speed tests after measurement errors

.gitignore

@@ -8,6 +8,7 @@
 /.playwright
 /blob-report
 /coverage
+/.vitest
 
 # Prevent temporary test pages from getting commited
 example/basic-page/*.html

README.md

@@ -58,7 +58,8 @@ them.
 | **measureUploadLoadedLatency**: *boolean* | Whether to perform additional latency measurements simultaneously with upload requests, to measure loaded latency (during upload). | `true` |
 | **loadedLatencyThrottle**: *number* | Time interval to wait in between loaded latency requests (in milliseconds). | 400 |
 | **bandwidthFinishRequestDuration**: *number* | The minimum duration (in milliseconds) to reach in download/upload measurement sets for halting further measurements with larger file sizes in the same direction. | 1000 |
-| **bandwidthAbortRequestDuration**: *number* | The minimum duration (in milliseconds) to reach in download/upload measurement sets for aborting the test early | 0 (disabled) |
+| **abortRequestDuration**: *number* | The maximum duration (in milliseconds) of a single request. Requests that take longer are aborted and skipped. `bandwidthAbortRequestDuration` is a deprecated alias. | 0 (disabled) |
+| **abortTestDuration**: *number* | The maximum running time (in milliseconds, excluding paused time) of the whole test. When it's reached, the test ends with the results collected so far. | 0 (disabled) |
 | **estimatedServerTime**: *number* | If the download/upload APIs do not return a `server-timing` response header containing the time spent in the server, this fixed value (in milliseconds) will be subtracted from all time-to-first-byte calculations. | 0 |
 | **latencyPercentile**: *number* | The percentile (between 0 and 1) used to calculate latency from a set of measurements. | 0.5 |
 | **bandwidthPercentile**: *number* | The percentile (between 0 and 1) used to calculate bandwidth from a set of measurements. | 0.9 |
@@ -88,8 +89,8 @@ them.
 | --- | --- | --- |
 | **onRunningChange** | running: *boolean* | Invoked whenever the test engine starts or stops. The current state is included as a function argument. |
 | **onResultsChange** | { type: *string* } | Invoked whenever any item changes in the results, usually indicating the completion of a measurement. The type of measurement that changed is included as an info attribute in the function argument. |
-| **onFinish** | results: *[Results](#results-object)* | Invoked whenever the test engine finishes all the measurements. The final [results object](#results-object) is included as a function argument. |
-| **onError** | error: *string* | Invoked whenever an error occurs during one of the measurements. The error details are included as a function argument. |
+| **onFinish** | results: *[Results](#results-object)* | Invoked when the test ends, with the [results object](#results-object) collected so far as a function argument. |
+| **onError** | error: *string*, status: *number \| undefined* | Invoked whenever an error occurs during one of the measurements, with the HTTP status when available. Most errors don't stop the test; call `pause()` from this callback to stop it. |
 
 ### Measurement config
 
@@ -131,7 +132,7 @@ Each of these measurement sets are bound to a specific file size. The engine fol
 | Field | Required | Description | Default |
 | --- | :--: | --- | :--: |
 | **bytes**: *number* | yes | The file size to request from the download API, or post to the upload API. The bandwidth (calculated as bits per second, or bps) for each request is calculated by dividing the `transferSize` (in bits) by the request duration (excluding the server processing time). Durations are measured from the final response headers where the browser reports them (`finalResponseHeadersStart`), so interim `1xx` responses do not shorten them. | - |
-| **count**: *number* | yes | The number of requests to perform for this file size. A request whose reported duration is zero or negative, or that has no resource timing entry, has no measurable bandwidth: it still counts towards `count`, but is discarded from the results with a console warning, so a set can return fewer data points than requested. If three requests in a row within one measurement set have no resource timing entry, the test stops and reports it through `onError`. | - |
+| **count**: *number* | yes | The number of requests to perform for this file size. A request whose reported duration is zero or negative, or that has no resource timing entry, has no measurable bandwidth: it still counts towards `count`, but is discarded from the results with a console warning, so a set can return fewer data points than requested. If three requests in a row within one measurement set have no resource timing entry, the test ends with the results collected so far, and reports it through `onError`. | - |
 | **bypassMinDuration**: *boolean* | no | Whether the `bandwidthMinRequestDuration` check should be ignored, and the engine is instructed to proceed with the measurements of this direction in any case. | `false` |
 
 #### packetLoss

src/Results/index.ts

@@ -36,11 +36,25 @@ export interface RawMeasurementEntry {
   [key: string]: unknown;
 }
 
-export interface RawResults {
+/** All measurement types the engine can run; each configured one has an entry in {@link RawResults}. */
+export type MeasurementType =
+  | 'latency'
+  | 'latencyUnderLoad'
+  | 'download'
+  | 'upload'
+  | 'packetLoss'
+  | 'packetLossUnderLoad'
+  | 'v4Reachability'
+  | 'v6Reachability'
+  | 'rpki'
+  | 'nxdomain';
+
+export type RawResults = {
   /** Excluding paused time (ms). */
   totalDurationMs: number | undefined;
-  [key: string]: RawMeasurementEntry | number | undefined;
-}
+  /** Why the test ended early, when it did (e.g. `abortTestDuration`). */
+  error?: string;
+} & { [type in MeasurementType]?: RawMeasurementEntry };
 
 // --- Re-exports for consumers ---
 
@@ -220,7 +234,7 @@ class Results {
   // Internal methods
   #calcGetter = (
     calcFn: keyof MeasurementCalculations,
-    resKey: string,
+    resKey: MeasurementType,
     defaultVal: unknown = undefined,
     surfaceError = false
     // eslint-disable-next-line @typescript-eslint/no-explicit-any

src/config/defaultConfig.ts

@@ -130,10 +130,17 @@ export interface Config {
   estimatedServerTime: number;
 
   /**
-   * Per-request duration (ms) at which to abort the current request and stop
-   * further rounds of that bandwidth type. Set to `0` to disable. Default: `0`.
+   * Maximum duration (ms) of a single request. Longer requests are aborted and
+   * skipped. `0` disables it. Default: `0`.
    */
-  bandwidthAbortRequestDuration: number;
+  abortRequestDuration: number;
+
+  /**
+   * Maximum running time (ms, excluding paused time) of the whole test. When
+   * reached, the test ends with the results collected so far. `0` or less
+   * disables it. Default: `0`.
+   */
+  abortTestDuration: number;
 
   /** Percentile used to compute latency from the sample set (0–1). Default: `0.5` (median). */
   latencyPercentile: number;
@@ -154,7 +161,10 @@ export interface Config {
  * override individual options; omitted properties fall back to their defaults
  * in {@link defaultConfig}.
  */
-export type ConfigOptions = Partial<Config>;
+export type ConfigOptions = Partial<Config> & {
+  /** @deprecated Use `abortRequestDuration`, which wins if both are set. */
+  bandwidthAbortRequestDuration?: number;
+};
 
 const REL_API_URL = 'https://speed.cloudflare.com';
 
@@ -212,7 +222,8 @@ const defaultConfig: Config = {
   estimatedServerTime: 0, // ms to discount from latency calculation (if not present in response headers)
 
   // Test abort
-  bandwidthAbortRequestDuration: 0, // download/upload duration (ms) to abort measurement early and stop further measurements of that type
+  abortRequestDuration: 0, // request duration (ms) after which a request is aborted and skipped
+  abortTestDuration: 0, // total running time (ms) to abort the whole test, keeping the results so far
 
   // Result interpretation
   latencyPercentile: 0.5, // Percentile used to calculate latency from a set of measurements

src/engines/BandwidthEngine/BandwidthEngine.ts

@@ -249,7 +249,13 @@ class BandwidthMeasurementEngine implements Engine {
   }
 
   finishRequestDuration: number = 1000; // download/upload duration (ms) to reach for stopping further measurements
-  abortRequestDuration: number = 0;
+  #abortRequestDuration: number = 0; // request duration (ms) after which a request is aborted and skipped
+  get abortRequestDuration(): number {
+    return this.#abortRequestDuration;
+  }
+  set abortRequestDuration(v: number) {
+    this.#abortRequestDuration = v;
+  }
   getServerTime: ((r: Response) => number | undefined) | null = cfGetServerTime; // method to extract server time from response
 
   #responseHook: (r: ResponseHookPayload) => void = () => {}; // pipe-through of response objects
@@ -290,19 +296,25 @@ class BandwidthMeasurementEngine implements Engine {
   set onConnectionError(f: (error: string, status?: number) => void) {
     this.#onConnectionError = f;
   }
+  #onFatalError: (error: string) => void = () => {}; // Invoked when the engine can't measure at all, after pausing itself
+  set onFatalError(f: (error: string) => void) {
+    this.#onFatalError = f;
+  }
 
   // Public methods
   pause(): void {
     if (this.#retryTimeout !== undefined) {
       clearTimeout(this.#retryTimeout);
       this.#retryTimeout = undefined;
     }
+    clearTimeout(this.#abortTimeout);
+    clearTimeout(this.#throttleTimeout);
     this.#cancelCurrentMeasurement(`pause()`);
     this.#setRunning(false);
   }
 
   play(): void {
-    if (!this.#failed && !this.#running) {
+    if (!this.#running) {
       this.#setRunning(true);
       this.#nextMeasurement();
     }
@@ -314,14 +326,15 @@ 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;
   #missingEntries: number = 0;
   #retryTimeout: ReturnType<typeof setTimeout> | undefined;
+  #abortTimeout: ReturnType<typeof setTimeout> | undefined;
+  #throttleTimeout: ReturnType<typeof setTimeout> | undefined;
   #minDuration: number = -Infinity; // of current measurement
   #throttleMs: number = 0;
   #estimatedServerTime: number = 0;
@@ -467,20 +480,16 @@ class BandwidthMeasurementEngine implements Engine {
     ) {
       // abort existing abort controller
       this.#currentAbortController?.abort('restarting engine');
+      clearTimeout(this.#abortTimeout);
 
       // create new abort controller
-      this.#currentAbortController = new AbortController();
+      const controller = (this.#currentAbortController = new AbortController());
       if (this.abortRequestDuration) {
-        const abortTimeout = setTimeout(() => {
-          const errorMessage = `${isDown ? 'Download' : 'Upload'} measurement of ${numBytes} bytes aborted. Measurement exceeded bandwidthAbortRequestDuration (${this.abortRequestDuration}ms)`;
-          this.#retries = 0;
-          this.pause();
-          this.#failed = true;
-          this.#onConnectionError(errorMessage);
+        this.#abortTimeout = setTimeout(() => {
+          const errorMessage = `${isDown ? 'Download' : 'Upload'} request of ${numBytes} bytes aborted. Request exceeded abortRequestDuration (${this.abortRequestDuration}ms)`;
+          controller.abort(errorMessage);
+          this.#skipFailedRequest(errorMessage);
         }, this.abortRequestDuration);
-        this.#currentAbortController.signal.addEventListener('abort', () =>
-          clearTimeout(abortTimeout)
-        );
       }
     }
 
@@ -536,8 +545,7 @@ class BandwidthMeasurementEngine implements Engine {
           );
           if (++this.#missingEntries >= MAX_MISSING_ENTRIES) {
             this.pause();
-            this.#failed = true;
-            this.#onConnectionError(
+            this.#onFatalError(
               `No resource timing entries for ${url}. The host page may be clearing or filling the buffer, or the runtime doesn't record resource timing.`
             );
             return;
@@ -681,7 +689,7 @@ class BandwidthMeasurementEngine implements Engine {
         ) {
           this.#retryTimeout = setTimeout(() => {
             this.#retryTimeout = undefined;
-            if (this.#running && !this.#failed) this.#nextMeasurement();
+            if (this.#running) this.#nextMeasurement();
           }, getRetryDelay(error.retryAfter));
           return;
         }
@@ -693,25 +701,33 @@ class BandwidthMeasurementEngine implements Engine {
             : status === undefined
               ? `Connection failed to ${url}.`
               : `Request failed with ${status}: ${url}`;
-        this.#retries = 0;
-        this.pause();
-        this.#failed = true;
-        this.#onConnectionError(message, status);
+        this.#skipFailedRequest(message, status);
       });
   }
 
+  // Reports a failed request and moves on, so one failure can't end the set
+  #skipFailedRequest(message: string, status?: number): void {
+    // An abort can land during a 429 backoff; its retry must not run too
+    clearTimeout(this.#retryTimeout);
+    this.#retryTimeout = undefined;
+    this.#retries = 0;
+    this.#onConnectionError(message, status);
+    this.#running && this.#scheduleNextMeasurement();
+  }
+
   #scheduleNextMeasurement(): void {
+    // The request has settled, so its abort timer must not fire during the throttle
+    clearTimeout(this.#abortTimeout);
     this.#counter += 1;
     this.#retries = 0;
 
     if (this.#throttleMs) {
-      const throttleTimeout = setTimeout(
-        () => this.#nextMeasurement(),
-        this.#throttleMs
-      );
-      this.#currentAbortController!.signal.addEventListener('abort', () =>
-        clearTimeout(throttleTimeout)
-      );
+      // Cleared by pause(): the controller may already be aborted, so its
+      // abort event can't cancel this
+      this.#throttleTimeout = setTimeout(() => {
+        this.#throttleTimeout = undefined;
+        if (this.#running) this.#nextMeasurement();
+      }, this.#throttleMs);
     } else {
       this.#nextMeasurement();
     }

src/engines/BandwidthEngine/ParallelLatency.ts

@@ -63,7 +63,6 @@ class BandwidthWithParallelLatencyEngine extends BandwidthEngine {
       };
 
       super.onRunningChange = this.#setLatencyRunning;
-      super.onConnectionError = () => this.#latencyEngine!.pause();
     }
   }
 
@@ -80,6 +79,11 @@ class BandwidthWithParallelLatencyEngine extends BandwidthEngine {
         f(res));
   }
 
+  // Called when a parallel latency request fails; both engines keep measuring
+  set onParallelLatencyError(f: (error: string, status?: number) => void) {
+    this.#latencyEngine && (this.#latencyEngine.onConnectionError = f);
+  }
+
   // Overridden attributes
   get fetchOptions(): RequestInit {
     return super.fetchOptions;
@@ -89,28 +93,31 @@ class BandwidthWithParallelLatencyEngine extends BandwidthEngine {
     this.#latencyEngine && (this.#latencyEngine.fetchOptions = fetchOptions);
   }
 
+  get abortRequestDuration(): number {
+    return super.abortRequestDuration;
+  }
+  set abortRequestDuration(abortRequestDuration: number) {
+    super.abortRequestDuration = abortRequestDuration;
+    this.#latencyEngine &&
+      (this.#latencyEngine.abortRequestDuration = abortRequestDuration);
+  }
+
+  set onFatalError(onFatalError: (error: string) => void) {
+    super.onFatalError = onFatalError;
+    this.#latencyEngine &&
+      (this.#latencyEngine.onFatalError = (error: string) => {
+        this.pause();
+        onFatalError(error);
+      });
+  }
+
   set onRunningChange(onRunningChange: (running: boolean) => void) {
     super.onRunningChange = (running: boolean) => {
       this.#setLatencyRunning(running);
       onRunningChange(running);
     };
   }
 
-  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
   #latencyEngine: BandwidthEngine | undefined;
   #latencyTimeout: ReturnType<typeof setTimeout> | undefined;

src/engines/Engine.ts

@@ -20,9 +20,12 @@ export interface Engine {
    */
   onFinished: (...args: any[]) => void;
 
-  /** Invoked when a connection-level error prevents measurement. */
+  /** Invoked when a request fails; the engine skips it and carries on. */
   onConnectionError?: (...args: any[]) => void;
 
+  /** Invoked when the engine can't measure at all and has paused itself. */
+  onFatalError?: (...args: any[]) => void;
+
   /** Invoked when a message is received (packet-loss engines). */
   onMsgReceived?: (...args: any[]) => void;
 
@@ -38,6 +41,9 @@ export interface Engine {
   /** Invoked for each parallel-latency sample collected alongside a bandwidth measurement. */
   onParallelLatencyResult?: (...args: any[]) => void;
 
+  /** Invoked when a parallel-latency request fails (bandwidth engines). */
+  onParallelLatencyError?: (...args: any[]) => void;
+
   pause?: () => void;
 
   play?: () => void;
@@ -55,8 +61,8 @@ export interface Engine {
   finishRequestDuration?: number;
 
   /**
-   * Maximum allowed request duration (ms). If a single request exceeds this
-   * value the engine aborts the measurement.
+   * Maximum allowed request duration (ms). A request that exceeds it is
+   * aborted and skipped.
    */
   abortRequestDuration?: number;
 }