diff --git a/src/engines/BandwidthEngine/BandwidthEngine.ts b/src/engines/BandwidthEngine/BandwidthEngine.ts index a3353df..40cf34a 100644 --- a/src/engines/BandwidthEngine/BandwidthEngine.ts +++ b/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 = { down: false, up: false }; #results: BandwidthEngineResults = { down: {}, up: {} }; #measIdx: number = 0; #counter: number = 0; #retries: number = 0; + #retryTimeout: ReturnType | 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); }); } diff --git a/src/engines/BandwidthEngine/ParallelLatency.ts b/src/engines/BandwidthEngine/ParallelLatency.ts index 6db7228..b29fef6 100644 --- a/src/engines/BandwidthEngine/ParallelLatency.ts +++ b/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 diff --git a/src/index.ts b/src/index.ts index d38ee3a..b419735 100644 --- a/src/index.ts +++ b/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!(); diff --git a/tests/e2e/httpError.test.ts b/tests/e2e/httpError.test.ts new file mode 100644 index 0000000..0d069ab --- /dev/null +++ b/tests/e2e/httpError.test.ts @@ -0,0 +1,253 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import SpeedTest from '../../src'; +import BandwidthEngine from '../../src/engines/BandwidthEngine/BandwidthEngine'; + +const originalFetch = window.fetch; + +afterEach(() => { + window.fetch = originalFetch; + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +describe.each([400, 401, 403, 408, 500])('bandwidth HTTP %i', status => { + it('stops the top-level engine without retrying, advancing, or resuming', async () => { + const fetchMock = vi.fn(() => + Promise.resolve(new Response(null, { status })) + ); + window.fetch = fetchMock; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const engine = new SpeedTest({ + autoStart: false, + downloadApiUrl: 'https://example.com/down', + uploadApiUrl: 'https://example.com/up', + logAimApiUrl: null, + logMeasurementApiUrl: null, + measurements: [ + { type: 'latency', numPackets: 1 }, + { type: 'download', bytes: 1_000, count: 1 } + ] + }); + const phases: string[] = []; + const resultErrors: string[] = []; + const onFinish = vi.fn(); + engine.onPhaseChange = ({ measurement }) => phases.push(measurement.type); + engine.onResultsChange = ({ type }) => { + const results = engine.results.raw[type]; + if (typeof results === 'object' && results.error) { + resultErrors.push(results.error); + } + }; + engine.onFinish = onFinish; + + const error = new Promise<{ + message: string; + running: boolean; + status?: number; + }>(resolve => { + engine.onError = (message, responseStatus) => { + resolve({ + message, + running: engine.isRunning, + status: responseStatus + }); + }; + }); + + engine.play(); + + await expect(error).resolves.toEqual({ + message: expect.stringMatching( + new RegExp(`^Request failed with ${status}: https://example\\.com/down`) + ), + running: false, + status + }); + await new Promise(resolve => setTimeout(resolve, 20)); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(phases).toEqual(['latency']); + expect(resultErrors).toEqual([ + expect.stringMatching(`^Request failed with ${status}:`) + ]); + expect(onFinish).not.toHaveBeenCalled(); + + engine.play(); + await new Promise(resolve => setTimeout(resolve, 20)); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); +}); + +describe.each([ + ['uses Retry-After', '2', 2_000], + ['waits five seconds when Retry-After is missing', null, 5_000] +])('HTTP 429 %s', (_label, retryAfter, delay) => { + it('retries three times before failing', async () => { + vi.useFakeTimers(); + const fetchMock = vi.fn(() => + Promise.resolve( + new Response(null, { + status: 429, + headers: retryAfter ? { 'retry-after': retryAfter } : undefined + }) + ) + ); + window.fetch = fetchMock; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const engine = new BandwidthEngine([{ dir: 'down', bytes: 0, count: 1 }], { + downloadApiUrl: 'https://example.com/down', + uploadApiUrl: 'https://example.com/up' + }); + const error = new Promise<{ message: string; status?: number }>(resolve => { + engine.onConnectionError = (message, responseStatus) => + resolve({ message, status: responseStatus }); + }); + + engine.play(); + + await vi.advanceTimersByTimeAsync(0); + for (let retry = 1; retry <= 3; retry++) { + await vi.advanceTimersByTimeAsync(delay - 1); + expect(fetchMock).toHaveBeenCalledTimes(retry); + await vi.advanceTimersByTimeAsync(1); + expect(fetchMock).toHaveBeenCalledTimes(retry + 1); + } + + await expect(error).resolves.toEqual({ + message: expect.stringContaining('Gave up after 3 retries.'), + status: 429 + }); + }); +}); + +it('cancels a pending 429 retry when paused', async () => { + vi.useFakeTimers(); + const fetchMock = vi.fn(() => + Promise.resolve( + new Response(null, { + status: 429, + headers: { 'retry-after': '1' } + }) + ) + ); + window.fetch = fetchMock; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const engine = new BandwidthEngine([{ dir: 'down', bytes: 0, count: 1 }], { + downloadApiUrl: 'https://example.com/down', + uploadApiUrl: 'https://example.com/up' + }); + + engine.play(); + await vi.advanceTimersByTimeAsync(0); + engine.pause(); + await vi.advanceTimersByTimeAsync(1_000); + + expect(fetchMock).toHaveBeenCalledTimes(1); +}); + +it('cancels the old engine 429 retry when restarted', async () => { + vi.useFakeTimers(); + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + new Response(null, { + status: 429, + headers: { 'retry-after': '1' } + }) + ) + .mockResolvedValue(new Response(null, { status: 401 })); + window.fetch = fetchMock; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const engine = new SpeedTest({ + autoStart: false, + downloadApiUrl: 'https://example.com/down', + uploadApiUrl: 'https://example.com/up', + logAimApiUrl: null, + logMeasurementApiUrl: null, + measurements: [{ type: 'download', bytes: 1_000, count: 1 }] + }); + + engine.play(); + await vi.advanceTimersByTimeAsync(0); + engine.restart(); + await vi.advanceTimersByTimeAsync(1_000); + + expect(fetchMock).toHaveBeenCalledTimes(2); +}); + +it('stops and cannot resume after a loaded-latency error', async () => { + const fetchMock = vi.fn( + (input: RequestInfo | URL, init?: RequestInit): Promise => { + const url = new URL(input.toString()); + if (url.searchParams.get('bytes') === '0') { + return Promise.resolve(new Response(null, { status: 401 })); + } + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => + reject(init.signal?.reason) + ); + }); + } + ); + window.fetch = fetchMock; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const engine = new SpeedTest({ + autoStart: false, + downloadApiUrl: 'https://example.com/down', + uploadApiUrl: 'https://example.com/up', + logAimApiUrl: null, + logMeasurementApiUrl: null, + measureDownloadLoadedLatency: true, + measurements: [{ type: 'download', bytes: 1_000, count: 1 }] + }); + const resultErrors: string[] = []; + engine.onResultsChange = ({ type }) => { + const results = engine.results.raw[type]; + if (typeof results === 'object' && results.error) { + resultErrors.push(results.error); + } + }; + const error = new Promise<{ running: boolean; status?: number }>(resolve => { + engine.onError = (_message, status) => { + resolve({ running: engine.isRunning, status }); + }; + }); + + engine.play(); + + await expect(error).resolves.toEqual({ running: false, status: 401 }); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(resultErrors).toEqual([expect.stringContaining('Request failed')]); + + engine.play(); + await new Promise(resolve => setTimeout(resolve, 20)); + expect(fetchMock).toHaveBeenCalledTimes(2); +}); + +it('stops immediately after a network failure', async () => { + const fetchMock = vi.fn(() => Promise.reject(new TypeError('fetch failed'))); + window.fetch = fetchMock; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const engine = new BandwidthEngine([{ dir: 'down', bytes: 0, count: 1 }], { + downloadApiUrl: 'https://example.com/down', + uploadApiUrl: 'https://example.com/up' + }); + const error = new Promise<{ message: string; status?: number }>(resolve => { + engine.onConnectionError = (message, status) => + resolve({ message, status }); + }); + + engine.play(); + + await expect(error).resolves.toEqual({ + message: expect.stringContaining('Connection failed'), + status: undefined + }); + expect(fetchMock).toHaveBeenCalledTimes(1); +});