fix: drop samples without a resource timing entry instead of stopping the test

README.md

@@ -131,7 +131,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 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. | - |
+| **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`. | - |
 | **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/engines/BandwidthEngine/BandwidthEngine.ts

@@ -67,6 +67,15 @@ export type InterimAwareTiming = PerformanceResourceTiming & {
 export const getFinalResponseStart = (perf: InterimAwareTiming): number =>
   perf.finalResponseHeadersStart || perf.responseStart;
 
+/** Missing resource timing entries in a row before giving up on a set. */
+const MAX_MISSING_ENTRIES = 3;
+
+/** Entries for `url` from requests started at or after `since`. */
+const getRequestEntries = (url: string, since: number) =>
+  performance
+    .getEntriesByName(url)
+    .filter(entry => entry.startTime >= since) as PerformanceResourceTiming[];
+
 /** Time to first byte: time from request start to the final response (ms). */
 const getTtfb = (perf: PerformanceResourceTiming): number =>
   getFinalResponseStart(perf) - perf.requestStart;
@@ -311,6 +320,7 @@ class BandwidthMeasurementEngine implements Engine {
   #measIdx: number = 0;
   #counter: number = 0;
   #retries: number = 0;
+  #missingEntries: number = 0;
   #retryTimeout: ReturnType<typeof setTimeout> | undefined;
   #minDuration: number = -Infinity; // of current measurement
   #throttleMs: number = 0;
@@ -397,8 +407,8 @@ class BandwidthMeasurementEngine implements Engine {
 
       // clear settings
       this.#counter = 0;
+      this.#missingEntries = 0;
       this.#minDuration = -Infinity;
-      performance.clearResourceTimings();
 
       do {
         this.#measIdx += 1; // skip through finished measurements
@@ -475,9 +485,11 @@ class BandwidthMeasurementEngine implements Engine {
     }
 
     let serverTime: number | undefined;
+    const signal = this.#currentAbortController!.signal;
+    const requestedAt = performance.now();
     fetch(url, {
       ...fetchOpt,
-      signal: this.#currentAbortController!.signal
+      signal
     })
       .then(r => {
         if (r.ok) return r;
@@ -502,10 +514,39 @@ class BandwidthMeasurementEngine implements Engine {
           return body;
         })
       )
-      .then(() => {
-        const perf = performance
-          .getEntriesByName(url)
-          .slice(-1)[0] as PerformanceResourceTiming; // get latest perf timing
+      .then(async () => {
+        // Every request in a set shares the URL, so skip earlier requests' entries
+        const entries = getRequestEntries(url, requestedAt);
+        if (entries.length) return entries;
+        // The entry can land a task after the body resolves, so look once more
+        await new Promise(resolve => setTimeout(resolve));
+        return getRequestEntries(url, requestedAt);
+      })
+      .then(entries => {
+        if (signal.aborted) return;
+
+        const [perf] = entries;
+        if (!perf) {
+          // A full or cleared buffer, or a runtime that doesn't record entries
+          console.warn(
+            `Discarding ${dir} sample without a resource timing entry`,
+            {
+              url
+            }
+          );
+          if (++this.#missingEntries >= MAX_MISSING_ENTRIES) {
+            this.pause();
+            this.#failed = true;
+            this.#onConnectionError(
+              `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;
+          }
+          this.#scheduleNextMeasurement();
+          return;
+        }
+        this.#missingEntries = 0;
+
         const timing: BandwidthMeasurementTiming = {
           transferSize: perf.transferSize,
           ttfb: getTtfb(perf),
@@ -622,30 +663,15 @@ class BandwidthMeasurementEngine implements Engine {
                 .finalResponseHeadersStart,
               responseEnd: perf.responseEnd,
               nextHopProtocol: perf.nextHopProtocol,
-              entries: performance.getEntriesByName(url).length
+              entries: entries.length
             }
           });
         }
 
-        this.#counter += 1;
-        this.#retries = 0;
-
-        if (this.#throttleMs) {
-          const throttleTimeout = setTimeout(
-            () => this.#nextMeasurement(),
-            this.#throttleMs
-          );
-          this.#currentAbortController!.signal.addEventListener('abort', () =>
-            clearTimeout(throttleTimeout)
-          );
-        } else {
-          this.#nextMeasurement();
-        }
+        this.#scheduleNextMeasurement();
       })
       .catch(error => {
-        if (this.#currentAbortController!.signal.aborted) {
-          return;
-        }
+        if (signal.aborted) return;
         console.warn(`Error fetching ${url}: ${error}`);
 
         if (
@@ -674,6 +700,23 @@ class BandwidthMeasurementEngine implements Engine {
       });
   }
 
+  #scheduleNextMeasurement(): void {
+    this.#counter += 1;
+    this.#retries = 0;
+
+    if (this.#throttleMs) {
+      const throttleTimeout = setTimeout(
+        () => this.#nextMeasurement(),
+        this.#throttleMs
+      );
+      this.#currentAbortController!.signal.addEventListener('abort', () =>
+        clearTimeout(throttleTimeout)
+      );
+    } else {
+      this.#nextMeasurement();
+    }
+  }
+
   #cancelCurrentMeasurement(reason?: string): void {
     this.#currentAbortController?.abort(
       reason || `aborted with no reason provided`

src/index.ts

@@ -196,9 +196,9 @@ class MeasurementEngine {
   /**
    * Starts or resumes the test.
    *
-   * On first call, clears the browser's resource timing buffer and begins
-   * the measurement sequence. On subsequent calls after {@link pause},
-   * resumes the current phase.
+   * Clears the browser's resource timing buffer on every start and resume,
+   * then begins the measurement sequence or, after {@link pause}, resumes the
+   * current phase.
    */
   play(): void {
     if (!this.#failed && !this.#running) {

tests/e2e/missingResourceTiming.test.ts

@@ -0,0 +1,57 @@
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import SpeedTest from '../../src/index.ts';
+
+const DOWNLOAD_URL = /\/__down\?(.*&)?bytes=100000(&|$)/;
+
+describe('SpeedTest E2E without a resource timing entry', () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it(
+    'drops the sample and finishes the test',
+    { timeout: 60_000 },
+    async () => {
+      // Hide the first download's entry, as when the host page clears the buffer
+      const getEntriesByName = performance.getEntriesByName.bind(performance);
+      let hiddenStart: number | undefined;
+      vi.spyOn(performance, 'getEntriesByName').mockImplementation(
+        (name, type) => {
+          const entries = getEntriesByName(name, type);
+          if (!DOWNLOAD_URL.test(name)) return entries;
+          hiddenStart ??= entries[0]?.startTime;
+          return entries.filter(entry => entry.startTime !== hiddenStart);
+        }
+      );
+
+      const engine = new SpeedTest({
+        autoStart: false,
+        logAimApiUrl: null,
+        logMeasurementApiUrl: null,
+        measureDownloadLoadedLatency: false,
+        measurements: [{ type: 'download', bytes: 1e5, count: 3 }]
+      });
+      const onError = vi.fn();
+      const warn = vi.spyOn(console, 'warn');
+
+      const results = await new Promise<typeof engine.results>(resolve => {
+        engine.onFinish = resolve;
+        // Settle on an error too, so a regression fails fast below
+        engine.onError = error => {
+          onError(error);
+          resolve(engine.results);
+        };
+        engine.play();
+      });
+
+      expect(hiddenStart).toBeDefined();
+      expect(warn).toHaveBeenCalledWith(
+        'Discarding down sample without a resource timing entry',
+        expect.objectContaining({ url: expect.stringMatching(DOWNLOAD_URL) })
+      );
+      expect(onError).not.toHaveBeenCalled();
+      expect(results.isFinished).toBe(true);
+      expect(results.getDownloadBandwidthPoints()).toHaveLength(2);
+    }
+  );
+});