[devandrepascoa]

Since 1.14, any bandwidth or loaded-latency error stops the whole test and onFinish never fires. On slow links, a large request that hits bandwidthAbortRequestDuration ends the test partway through.

Errors now affect only the request that failed. A failed download, upload or latency request (HTTP error including 401/403, network error, an abortRequestDuration abort, or a loaded-latency ping, which is now aborted too) is reported through onError with its status and skipped, and the step carries on with its next request. No step, direction or test ends early because of a failed request, so later steps always run, and loaded latency keeps sampling after a failed ping. A failed packet loss measurement still records its error and the test moves on. Three missing resource timing entries in a row still end the test early, as in #170, since the runtime or host page can't be measured at all. Unlike in #170, onFinish now fires with the results so far, but they aren't submitted to logAimApiUrl.

abortTestDuration (off by default) is what ends a test that runs too long. It counts running time only, aborts any download, upload or latency request in flight and keeps the results collected so far. results.raw.error and the measurement that was running get the reason, onError is called, then onFinish fires and results are still submitted to logAimApiUrl. With abortRequestDuration set, it's what caps total time: once one size aborts, larger sizes usually abort too, each request waiting the full abort duration.

Consumers that want to stop on an error, for example on a 401/403 using the status argument, can call pause() from onError; play() retries the failed request. Calling restart() from onError no longer lets the old run advance the new one, and a packet loss phase that finishes while paused no longer starts the next phase. Only 429 is retried, honouring Retry-After, as in 1.14.

Release note: error behaviour changes again for 1.14 consumers. Failed requests are skipped instead of stopping the test, onError can fire once per failed request, and its messages use the 1.13 format (e.g. Connection error while measuring download: …). bandwidthAbortRequestDuration is renamed to abortRequestDuration, since it applies to every request, loaded-latency pings included. The old name still works as a deprecated alias, and the new one wins if both are set. It now aborts only the request that exceeded it; set abortTestDuration to cap the total test time. results.raw is now typed with one optional key per measurement type instead of a string index signature, so TypeScript code that indexes it with an arbitrary string needs to use a MeasurementType key (e.g. 'download').

Fixes #163

[ask-bonk]

Review: 1 findings.

Restores continuation after measurement errors, supports pausing from onError, and prevents loaded-latency failures from stopping bandwidth tests. Posted 1 inline correctness finding.

Tests were not run because pnpm is unavailable in the runner.

github run

[devandrepascoa]

/ask-bonk

[clanky-the-clanker]

This pull request's reviews are posted as pull request reviews: see the review of f72d4654.

[devandrepascoa]

/ask-bonk

[devandrepascoa]

/ask-bonk

[devandrepascoa]

/ask-bonk

[clanky-the-clanker]

Superseded by the review of e2a6c430.

This review, of f72d4654

💬 Approved with comments — The code changes are sound, with no correctness, security or performance problems. Two INFO notes remain: the onError docs describe the pause/retry behaviour and where errors are recorded a little too broadly.

Findings: 2 info

Not opened by the correctness review: 1 changed file it was meant to read

• src/config/defaultConfig.ts

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 2 info

The onError docs describe pause/retry and error recording more broadly than the code behaves.

Recommendation: 💬 Optional Nits

• [INFO] The onError row says play() retries the failed request after a pause from onError. A failed packet loss measurement has already been marked finished, so play() moves on to the next measurement instead. Limit the retry wording to download/upload/latency requests. (README.md:93)
• [INFO] The onError JSDoc says the error is recorded on that measurement. Failed bandwidth and latency requests are only reported through the callback; only packet loss errors and the abortTestDuration abort go into results. Narrow the wording. (src/index.ts:182)

View full review

The earlier note on the abortTestDuration JSDoc is fixed.

• README.md:93: the onError row says "Call pause() from this callback… play() then retries the failed request." That is true for download, upload and latency requests. A packet loss measurement that failed has already been marked finished, though, so play() moves on to the next measurement instead of retrying it. Limit the retry sentence to request failures.
• src/index.ts:181-183: the onError JSDoc says "The test records the error on that measurement and continues." Failed bandwidth and latency requests are only reported through onError; they are not stored on the measurement result. Only packet loss errors and the abortTestDuration abort are written to results. Narrow the wording so the generated .d.ts stays accurate.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

No security issues. The change only affects how errors move the test along, plus the opt-in abortTestDuration timer.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

The changes since the last review hold up:

• bandwidthAbortRequestDuration timer: the timer aborts the request's controller and then skips the request. The fetch's .catch returns early when signal.aborted is set, so each failure is reported once, not twice. #setRunning(false) also aborts the controller, so the timer can't fire after a step has finished.
• Pause from onError: the next request is only scheduled if #running is still true after the callback. A pause() from onError therefore leaves the request counter where it is, and play() retries the same request.
• Loaded-latency errors: the latency engine is throttled, so a ping that keeps failing reports errors at that rate and doesn't spin. onParallelLatencyError ignores errors that arrive after the step has changed or the test has restarted.
• Tests: the unit test for missing resource timing entries now expects the engine to carry on after the error, which matches the new behaviour.

---

⚡ Performance: ✅ LGTM

No performance issues.

---

_{Reviewed commit: f72d4654}

[devandrepascoa]

/ask-bonk

[clanky-the-clanker]

Superseded by the review of 3efd729e.

This review, of e2a6c430

💬 Approved with comments — No correctness, security or performance problems found. The only open item is an INFO note on the README's onError retry wording.

Findings: 1 info

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

One README sentence about retrying after a pause is still a little broad

Recommendation: 💬 Optional Nits

• [INFO] The onError row says play() retries the failed request after a pause from onError. A failed packet loss measurement isn't retried: resuming moves on to the next measurement. Limit the retry wording to download, upload and latency requests. (README.md:93)

View full review

The onError JSDoc note from the last review is fixed: it now says only that a failed request is skipped (src/index.ts:180-184).

• README.md:93: the onError row now says that a failed packet loss measurement moves on to the next measurement. The next sentence, "play() then retries the failed request", still reads as if it covers every error. Pausing after a packet loss failure and resuming moves on to the next measurement instead of retrying. Saying "retries the failed download, upload or latency request" would make this clear.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The description includes a release note on the error behaviour change and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No security issues. The change only affects how request errors move the test along, plus the opt-in abortTestDuration timer.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

• Skipping failed requests: #skipFailedRequest (BandwidthEngine.ts) clears any pending 429 retry and resets the retry counter. It then reports the error and schedules the next request only if the engine is still running. A pause() from onError therefore leaves the request counter where it is, and play() retries the same request. A restart() from onError is safe because the old engine is already paused by then.
• bandwidthAbortRequestDuration timer: it aborts the controller and then skips the request. The fetch's .catch returns early when signal.aborted is set, so each failure is reported once.
• Fatal path: #failTest sets #failed before pausing, so play() does nothing until restart() clears it.
• abortTestDuration: the timer is rescheduled on every running change, so it counts running time only. #abortTest guards against a restart() from onError.
• Packet loss: each callback now checks that the test is still running before it calls #next(), so a phase that finishes while paused no longer starts the next phase. The new tests in tests/unit/packetLossFlow.test.ts cover the paused case and a restart() from onError.

---

⚡ Performance: ✅ LGTM

• Every failed request advances the request counter, so a step that keeps failing still finishes after a set number of requests, and no timers are left behind.
• The loaded-latency engine is throttled, so a ping that keeps failing reports errors at the normal sampling rate instead of spinning.
• The RawResults change only affects types.

---

_{Reviewed commit: e2a6c430}

[clanky-the-clanker]

Superseded by the review of cc608e15.

This review, of 3efd729e

✅ Approved — The only change since the last review fixes the README note it raised about onError retries. No new issues found.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low. The description includes a release note on the change in error behaviour and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No security issues found.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

Only README.md changed since the last review. In BandwidthEngine.ts, the bandwidthAbortRequestDuration timer is cleared when its controller aborts, which happens when the next request starts or the engine pauses. A timer left over from a finished request therefore can't report a spurious error.

---

⚡ Performance: ✅ LGTM

• A failed request still advances the request counter, so a step that keeps failing ends after a fixed number of requests.
• #skipFailedRequest clears any pending 429 retry timer, so an aborted request can't leave a stray timer or send a duplicate request.
• The RawResults change only affects types.

---

📝 Documentation: ✅ LGTM

README.md: the onError row now says that play() retries the failed download, upload or latency request, so it no longer suggests that a failed packet loss measurement is retried.

---

_{Reviewed commit: 3efd729e}

[clanky-the-clanker]

Superseded by the review of 4b61c260.

This review, of cc608e15

💬 Approved with comments — No defects found. The only item is an optional suggestion: say in the docs that bandwidthAbortRequestDuration doesn't cover loaded-latency pings.

Findings: 1 suggestion

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 suggestion

Optional: say that loaded-latency pings aren't covered by bandwidthAbortRequestDuration.

Recommendation: 💬 Optional Nits

• [SUGGESTION] The bandwidthAbortRequestDuration docs could say it applies to the latency step and to download/upload requests, but not to loaded-latency pings. The parallel latency engine in ParallelLatency.ts:43-60 is never given abortRequestDuration. (README.md:61)

View full review

README.md:61 and src/config/defaultConfig.ts:133-135 say bandwidthAbortRequestDuration caps "a single download, upload or latency request". In the code, src/index.ts:620-621 sets it on the standalone latency engine, and src/index.ts:708-709 sets it on the download/upload engines. The parallel loaded-latency engine built in src/engines/BandwidthEngine/ParallelLatency.ts:43-60 never gets it, so loaded-latency pings aren't capped. The README usually says "loaded latency" when it means those pings, so the current wording is close enough. Saying it outright would help users with slow connections.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The description's release note covers the change in error behaviour, abortTestDuration and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No exploitable problems. The change is about error handling and adds the abortTestDuration option. It adds no new injection points, secrets or authorization changes.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

No issues in the changes since the last review.

---

⚡ Performance: ✅ LGTM

No performance issues.

---

_{Reviewed commit: cc608e15}

[clanky-the-clanker]

Superseded by the review of f78df66d.

This review, of 4b61c260

💬 Approved with comments — No defects found. The only item is an optional suggestion to say in the docs that bandwidthAbortRequestDuration doesn't cover loaded-latency pings.

Findings: 1 suggestion

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 suggestion

One optional clarification about the per-request timeout option.

Recommendation: 💬 Optional Nits

• [SUGGESTION] Say that bandwidthAbortRequestDuration applies to the latency step and to download/upload requests, but not to loaded-latency pings: the parallel latency engine never gets abortRequestDuration. (README.md:61)

View full review

README.md:61 says bandwidthAbortRequestDuration caps "a single download, upload or latency request". The standalone latency engine and the download/upload engines get this timeout. The parallel loaded-latency engine in src/engines/BandwidthEngine/ParallelLatency.ts doesn't, so loaded-latency pings aren't capped. Saying this outright would help users on slow connections.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The description's release note covers the change in error behaviour, abortTestDuration, onFinish without logging after missing resource timing entries, and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No new injection points or secrets, and no changes to authorization. Handling of the authorization token is unchanged. Results from a test that ended after missing resource timing entries are no longer submitted to logAimApiUrl, which is the safer direction.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

• Fatal path: after three missing resource timing entries, #endTest(..., { skipLog: true }) destroys the current engine, records the error on raw.error and on the measurement that was running, and calls onError. It then returns early if onError restarted the test. Otherwise it finishes the run without submitting results. The abortTestDuration path still submits results.
• #destroyCurEngine clears onFatalError, onParallelLatencyError and the other callbacks, so late callbacks from an ended or restarted run do nothing.
• A play() after #endTest goes through #next() straight to a finish that does nothing.
• The new e2e test for this path checks the single onError, raw.error, isFinished, the fetch count and that nothing is logged.

---

⚡ Performance: ✅ LGTM

No performance issues.

---

_{Reviewed commit: 4b61c260}

[clanky-the-clanker]

Superseded by the review of 5b107d61.

This review, of f78df66d

✅ Approved — No defects found in the changes since the last review; the fatal-path logging, restart guards and the narrower results.raw type are consistent and covered by tests.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low. The release note in the description covers the change in error behaviour, abortTestDuration and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No new injection points or secrets, and nothing changes authorization or the handling of the authorization token.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

• #destroyCurEngine clears every engine callback, so late callbacks from an ended or restarted run do nothing.
• #endTest sets skipLog before calling onError and checks #measurementId again afterwards, so restart() from onError doesn't finish the old run.
• #setFinished keeps the skipLog value it had when it scheduled onFinish, so a pause() + play() from onError still finishes without submitting results.
• #skipFailedRequest clears any pending 429 retry and moves on only if the engine is still running. A pause() from onError leaves the counter unchanged, so play() retries the failed request.
• RawResults is now typed per MeasurementType, defined once in Results.

---

⚡ Performance: ✅ LGTM

Each failed request is reported once and schedules one next measurement. The abort listener clears its timer. A persistent failure such as a 401 can only skip through the step's finite list of requests, so the extra work is bounded.

---

📝 Documentation: ✅ LGTM

No new documentation issues. One earlier note is still open: the README doesn't say that bandwidthAbortRequestDuration leaves loaded-latency pings uncapped.

---

_{Reviewed commit: f78df66d}

[clanky-the-clanker]

Superseded by the review of 18167f9a.

This review, of 5b107d61

💬 Approved with comments — One WARNING: with abortRequestDuration set below ping time plus loadedLatencyThrottle, the abort timer fires after a successful loaded-latency ping and reports a spurious onError. The setting is off by default, and the rest of the change is consistent.

Findings: 1 warning (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #169.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Code Quality] The abortRequestDuration timer is only cleared on abort, so after a successful loaded-latency ping it keeps running through the loadedLatencyThrottle wait. It can then abort a completed ping, fire a spurious onError and skip a ping cycle.
- **File:** src/engines/BandwidthEngine/BandwidthEngine.ts:477

---

📋 Code Quality: ⚠️ 1 warning

The abort timer outlives a successful loaded-latency ping and can report it as aborted during the throttle wait.

Recommendation: 📝 Minor Comments

• [WARNING] The abortRequestDuration timer is only cleared on abort, so after a successful loaded-latency ping it keeps running through the loadedLatencyThrottle wait. It can then abort a completed ping, fire a spurious onError and skip a ping cycle. (src/engines/BandwidthEngine/BandwidthEngine.ts:477)

View full review

Spurious abort of a ping that already completed (src/engines/BandwidthEngine/BandwidthEngine.ts:475-485, 710-724)

The abortRequestDuration timer is cleared only by the controller's abort listener. A request that succeeds does not abort its controller. With throttleMs = 0, the main engine aborts the old controller right away in #nextMeasurement() ('restarting engine'), so the timer is cleared there. The loaded-latency engine is different: it runs with throttleMs = loadedLatencyThrottle (default 400), and its controller stays alive during the throttle wait. That window is new, because abortRequestDuration is now copied to the latency engine (ParallelLatency.ts:126).

If abortRequestDuration is shorter than the ping time plus the throttle, the timer fires after the ping has succeeded. It then:

• aborts the controller, which also clears the pending throttle timeout;
• calls #skipFailedRequest('Download request of 0 bytes aborted…'), so onError fires for a ping that succeeded, possibly on every cycle;
• bumps the counter again for a request that was not skipped.

The new e2e test uses loadedLatencyThrottle: 10 with abortRequestDuration: 100, so it never reaches this window.

Fix: clear the timer when the request settles (success or failure), or have the callback return early when the request is no longer in flight. Add a test with abortRequestDuration below loadedLatencyThrottle where every ping succeeds, and assert that onError is not called.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The release note in the description covers the rename to abortRequestDuration with its deprecated alias, the change in error behaviour and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No new injection points or secrets, and nothing changes authorization or the handling of the authorization token.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

⚡ Performance: ✅ LGTM

Each failed request is reported once and schedules one next measurement. The latency engine no longer pauses the bandwidth engine, and that adds no polling. abortRequestDuration is copied once each time the pings start.

---

📝 Documentation: ✅ LGTM

The README now says that abortRequestDuration applies to every request and that bandwidthAbortRequestDuration is a deprecated alias. This resolves the earlier note about loaded-latency pings.

---

_{Reviewed commit: 5b107d61}

[clanky-the-clanker]

⚠️ WARNING · Code Quality

The abortRequestDuration timer is only cleared on abort, so after a successful loaded-latency ping it keeps running through the loadedLatencyThrottle wait. It can then abort a completed ping, fire a spurious onError and skip a ping cycle.

This timer is cleared only when the controller aborts, and a successful request never aborts it. The loaded-latency engine waits loadedLatencyThrottle (default 400 ms) before #nextMeasurement() replaces the controller. So when abortRequestDuration is shorter than ping time plus throttle, the timer fires after the ping has succeeded. It aborts the controller, which cancels the throttle wait, and calls #skipFailedRequest, so onError reports an abort for a ping that succeeded and the counter moves on twice. Clear the timer when the fetch settles, or skip the callback once the request is no longer in flight.

[devandrepascoa]

AI Code Reviewer finding:

The abortRequestDuration timer is only cleared on abort, so after a successful loaded-latency ping it keeps running through the loadedLatencyThrottle wait. It can then abort a completed ping, fire a spurious onError and skip a ping cycle.

Local Agent verdict:

Valid, fixed in d895c75. The abort timer is now kept in a field and cleared when the request settles and on pause. The throttle timer is also cleared on pause, which fixes a related leak where an aborted ping's throttle survived the step and pings kept running. Added e2e tests for both cases.

[clanky-the-clanker]

Superseded by the review of 746b5785.

This review, of 18167f9a

✅ Approved — The one earlier WARNING, a spurious abort of a loaded-latency ping that had already succeeded, is fixed: the abort timer is now cleared when a request settles and on pause(), and a regression test covers it. No new issues were found.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low. The release note in the description covers the change in error behaviour, the rename to abortRequestDuration with its deprecated alias, and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No new injection points or secrets, and nothing changes authorization or how the authorization token is handled.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

🔄 Since last review: 1 finding resolved, 0 preserved, 0 new

Fixed: the abort timer outlived a settled ping.

• #scheduleNextMeasurement() now clears #abortTimeout before the throttle wait starts (src/engines/BandwidthEngine/BandwidthEngine.ts:720). Every settle path goes through it.
• pause() clears both #abortTimeout and #throttleTimeout (lines 310-311), so the throttle timer of an aborted ping no longer keeps running after pause().
• #nextMeasurement() clears any old abort timer when it starts a new request (line 483). The abort callback only aborts the controller it captured.
• The e2e test "does not abort a loaded-latency ping that succeeded during the throttle wait" uses abortRequestDuration 100 with loadedLatencyThrottle 200 and checks that onError is not called.

The rest of the changed code is consistent:

• The abortRequestDuration / bandwidthAbortRequestDuration resolution uses ?? on the user config, so the new name wins and 0 is respected.
• The #measurementId guards stop a restart() from onError from advancing the old run.
• #skipLog is set before onError runs, so resuming from onError still skips the results log.

---

⚡ Performance: ✅ LGTM

Each of the new timers lives in a single field and is cleared before it is replaced, so timers do not pile up. Each failed request schedules exactly one next measurement.

---

📝 Documentation: ✅ LGTM

No issues found.

---

_{Reviewed commit: 18167f9a}

[clanky-the-clanker]

Superseded by the review of 15fe79f5.

This review, of 746b5785

💬 Approved with comments — The config alias change is correct and no correctness, security or performance issues were found. Two INFO notes remain: the README's onFinish description and some Engine interface comments are out of date.

Findings: 2 info

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 2 info

The README's onFinish description and some Engine interface comments don't describe the new early-end and per-request error behaviour.

Recommendation: 💬 Optional Nits

• [INFO] The onFinish description says it fires when all measurements finish, but it now also fires when the test ends early (the abortTestDuration abort, or three missing resource timing entries in a row). Consider saying it fires whenever the test ends. (README.md:92)
• [INFO] The Engine interface comments still say a connection error prevents measurement and that abortRequestDuration aborts the measurement. Now only the failed or too-slow request is aborted and skipped. (src/engines/Engine.ts:64)

View full review

• README.md:92: the description says onFinish fires when the engine "finishes all the measurements". With this change it also fires when the test ends early: when abortTestDuration is reached, and after three missing resource timing entries in a row. Consider saying it fires whenever the test ends, with the results collected so far.
• src/engines/Engine.ts:23,64-65: two comments describe the old behaviour. onConnectionError is described as an error that "prevents measurement", and abortRequestDuration as aborting "the measurement". Failed requests are now skipped and the step carries on, and abortRequestDuration aborts only the request that ran too long.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The release note in the description covers the change in error behaviour, the rename to abortRequestDuration with its deprecated alias, and the narrower results.raw type.

---

🔒 Security: ✅ LGTM

No new injection points or secrets, and nothing changes how the authorization token is handled.

---

📖 AGENTS.md: ✅ LGTM

The change follows the repository's conventions: #field private fields, browser-only APIs and no new runtime dependencies.

---

📋 Code Quality: ✅ LGTM

• Alias resolution (src/index.ts:112-124): The constructor pulls bandwidthAbortRequestDuration out of the user config before merging it. It then sets abortRequestDuration with abortRequestDuration ?? bandwidthAbortRequestDuration ?? default, so the new name wins and an explicit 0 is respected. No other code in src/ reads the deprecated key.
• Types (src/config/defaultConfig.ts): The alias exists only on ConfigOptions, marked @deprecated. Config and defaultConfig hold abortRequestDuration alone.
• Tests: tests/unit/config/abortRequestDuration.test.ts covers the default, the new name, the alias, precedence including 0, and that the deprecated key is absent from the resolved config.

---

⚡ Performance: ✅ LGTM

• Each engine keeps its abort timer and its throttle timer in one field each. Both are cleared before they are replaced and when the engine pauses, so timers do not pile up.
• Each failed request schedules exactly one next measurement, and failed loaded-latency pings are still paced by loadedLatencyThrottle, so a failure cannot cause a tight retry loop.

---

_{Reviewed commit: 746b5785}

[clanky-the-clanker]

💬 Approved with comments — No correctness, security or performance issues found, and both earlier documentation notes are fixed. One INFO remains: the README's isFinished description doesn't mention that a test can end early.

Findings: 1 info

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

Earlier notes fixed; the README's isFinished description still says all measurements finished

Recommendation: 💬 Optional Nits

• [INFO] The isFinished description says all measurements have finished, but the test can now end early (the abortTestDuration abort, or three missing resource timing entries in a row). Consider saying the test has ended and the results are final. (README.md:76)

View full review

Both earlier notes are fixed. README.md:92 now says onFinish fires whenever the test ends, and the Engine.ts comments describe the skip-and-continue behaviour.

• README.md:76: the isFinished description says the engine has finished all the measurements. With abortTestDuration, or after three missing resource timing entries in a row, the test can end before every configured measurement has run. Consider saying instead that the test has ended and the results are final.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The release note in the description covers the behaviour change, the option rename with its deprecated alias, and the narrower results.raw typing.

---

🔒 Security: ✅ LGTM

No security concerns in the changes since the last review.

---

📖 AGENTS.md: ✅ LGTM

The changes follow the repository's conventions.

---

📋 Code Quality: ✅ LGTM

No findings. The updated Engine.ts comments for onConnectionError, onFatalError and abortRequestDuration now match the code. I re-checked how BandwidthEngine skips a failed request:

• An abort is reported only once. The fetch .catch returns early when the signal is already aborted.
• pause() from onError stops anything new from being scheduled, and play() retries the same request.
• When a request is skipped, any 429 retry still pending for it is cleared.

---

⚡ Performance: ✅ LGTM

No performance concerns.

---

_{Reviewed commit: 15fe79f5}