Skip to content

refactor: unify browser error classification and deduplicate retry logic#908

Merged
jackwener merged 2 commits intomainfrom
refactor/unify-error-classification-and-retry
Apr 9, 2026
Merged

refactor: unify browser error classification and deduplicate retry logic#908
jackwener merged 2 commits intomainfrom
refactor/unify-error-classification-and-retry

Conversation

@jackwener
Copy link
Copy Markdown
Owner

Summary

  • Replace two overlapping error classification systems (isTransientBrowserError + isRetryableSettleError) with a single classifyBrowserError() that returns { retryable, delayMs }
  • Deduplicate sendCommand() / sendCommandFull() retry loop (44 lines of identical code) into sendCommandRaw()

Design change

Before: two separate systems answering the same question ("is this error transient?") with different pattern lists, different delay strategies, in different files.

After: one function, one pattern list, one place to maintain. Each error category carries its own retry delay:

Category Patterns Delay
Extension/daemon transient Extension disconnected, attach failed, No window with id, etc. 1500ms
CDP target navigation Inspected target navigated or closed, -32000 + target 200ms
Non-transient Everything else not retryable

Files changed

File What changed
src/browser/errors.ts New classifyBrowserError()RetryAdvice; isTransientBrowserError() kept as convenience wrapper
src/browser/daemon-client.ts Extract sendCommandRaw(), uses classifyBrowserError().delayMs instead of hardcoded 1500ms
src/browser/page.ts Remove isRetryableSettleError(), use classifyBrowserError() with its delay advice
src/browser/errors.test.ts Tests for all error categories with delay assertions
src/browser.test.ts Updated to test unified classification

Test plan

  • 594 tests pass, 0 failures
  • TypeScript compiles clean
  • isTransientBrowserError() still works (backward compat wrapper)
  • Pipeline executor unchanged — still imports isTransientBrowserError from errors.ts

Replace two overlapping error classification systems with a single
classifyBrowserError() that returns retry advice (retryable + delayMs):

- Extension/daemon transient errors → retryable, 1500ms delay
- CDP target navigation errors → retryable, 200ms delay
- Non-transient errors → not retryable

Deduplicate sendCommand/sendCommandFull retry loop into sendCommandRaw,
making both public functions thin return-value wrappers.
classifyBrowserError() now returns a `kind` field:
- extension-transient: retried by daemon-client only
- target-navigation: retried by page-level settle logic
- non-retryable: no retry

Page.goto() and Page.evaluate() now only settle-retry on
target-navigation, preventing extension/daemon errors from being
silently swallowed as settle noise.
@jackwener jackwener merged commit bbd1163 into main Apr 9, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant