Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 5 additions & 45 deletions chrome-extension/src/background/chains/ethereumHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import * as wallet from '../wallet';
import { buildFeeWarning, getFeeFloor, getPriorityFeeFloor, type FeeChoice, type FeeWarning } from './feeFloors';
import { openSidePanel, setApprovalBadge } from '../popup';
import { getChainInfo, makeStaticProvider } from './registry';
import { isTransientRpcError } from './rpcFailover';
import { getLastResortRpcs } from './lastResortRpcs';

const TAG = ' | ethereumHandler | ';
Expand Down Expand Up @@ -1610,50 +1611,6 @@ async function getCandidateRpcs(): Promise<{
return { availableRpcs, networkId, chainIdRaw: currentProvider.chainId ?? '' };
}

/**
* Heuristic: is this RPC error worth retrying against a different URL?
* Used by withRpcFailover (read calls). Broadcast has its own
* classifier because it has additional tx-level definitive cases
* (insufficient funds, nonce too low, etc.).
*
* Includes "method-rejection" patterns because narrow-purpose RPCs in
* Pioneer's catalog (Flashbots' rpc.flashbots.net is the canonical
* example — only supports eth_sendRawTransaction / eth_chainId /
* eth_blockNumber, rejects everything else with HTTP 403 + JSON-RPC
* code -32601 "rpc method is not whitelisted") would otherwise be
* sticky: their pre-flight `getBlockNumber()` test passes, so they get
* picked first on every read, and every read fails 403. Treating the
* rejection as transient lets the loop blacklist them for 60s and try
* the next URL.
*/
const isTransientRpcError = (errMsg: string): boolean => {
const m = errMsg.toLowerCase();
return (
m.includes('rate limit') ||
m.includes('throttle') ||
m.includes('429') ||
m.includes('timeout') ||
m.includes('econnreset') ||
m.includes('etimedout') ||
m.includes('network') ||
m.includes('server_error') ||
m.includes('exceeded maximum retry') ||
/\b5\d{2}\b/.test(m) || // 5xx HTTP code
// Method-rejection: this URL doesn't support this method. Try next.
m.includes('rpc method is not whitelisted') ||
m.includes('method not found') ||
m.includes('method not supported') ||
m.includes('method does not exist') ||
m.includes('-32601') ||
// Narrow to ethers' transport-level wrapper text. A bare `.includes('403')`
// would misfire on revert reasons or hex payloads that happen to
// contain "403", causing a successfully-rejected eth_call to be
// replayed across every URL and pointlessly cool them all.
m.includes('server response 403') ||
m.includes('http 403')
);
};

/**
* Run a read-style RPC call across the failover candidate list. Used
* for preflight calls (nonce, gas estimate, fee data) where any working
Expand Down Expand Up @@ -1683,7 +1640,10 @@ async function withRpcFailover<T>(
const errMsg = String(e?.message || e);
if (!isTransientRpcError(errMsg)) {
// Definitive (revert, invalid params, etc.) — won't help to
// try another RPC. Surface to caller.
// try another RPC. Surface to caller. Log the URL first: this
// branch used to throw silently while the transient branch below
// logged, so an RPC failure left no trace and got misdiagnosed.
console.error(tag, `RPC ${url} definitive failure, aborting failover:`, errMsg);
throw e;
}
console.warn(tag, `RPC ${url} transient failure, trying next:`, errMsg);
Expand Down
18 changes: 18 additions & 0 deletions chrome-extension/src/background/chains/rpcFailover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,24 @@ describe('isTransientRpcError', () => {
expect(isTransientRpcError('503 Service Unavailable')).toBe(true);
});

// Regression: written against Firefox/Node wording only, so a dead RPC
// hard-threw on Chrome instead of failing over — and the resulting error
// was mislabeled "KeepKey Vault is not running".
it('classifies browser connection-level failures as transient', () => {
expect(isTransientRpcError('Failed to fetch')).toBe(true);
expect(isTransientRpcError('TypeError: Failed to fetch')).toBe(true);
expect(isTransientRpcError('Load failed')).toBe(true);
expect(isTransientRpcError('net::ERR_NAME_NOT_RESOLVED')).toBe(true);
expect(isTransientRpcError('signal is aborted without reason')).toBe(true);
expect(isTransientRpcError('NetworkError when attempting to fetch resource.')).toBe(true);
});

it('classifies method-rejection (Flashbots-style narrow RPCs) as transient', () => {
expect(isTransientRpcError('rpc method is not whitelisted')).toBe(true);
expect(isTransientRpcError('server response 403 Forbidden')).toBe(true);
expect(isTransientRpcError('the method does not exist/is not available')).toBe(true);
});

it('treats definitive RPC errors (revert / invalid params) as NOT transient', () => {
expect(isTransientRpcError('execution reverted')).toBe(false);
expect(isTransientRpcError('invalid params')).toBe(false);
Expand Down
46 changes: 44 additions & 2 deletions chrome-extension/src/background/chains/rpcFailover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,28 @@ const FAILED_RPC_COOLDOWN_MS = 60_000;
// blocking the other.
const failedRpcs = new Map<string, number>();

/**
* Heuristic: is this RPC error worth retrying against a different URL?
* Shared by both failover loops (this module's by-networkId reads and
* ethereumHandler's active-provider path) so the two cannot drift.
* Broadcast keeps its own classifier — it has tx-level definitive cases
* (insufficient funds, nonce too low) that don't apply to reads.
*
* The `failed to fetch` / `load failed` / `err_` group matters more than it
* looks: those are what Chrome and Safari throw for a connection-level
* failure (TLS handshake, DNS, refused). The original list was written
* against Firefox/Node wording ("NetworkError..."), so the same dead RPC
* failed over on Firefox and hard-threw on Chrome — and that hard throw
* reached a catch-all that mislabeled it "KeepKey Vault is not running".
*
* The method-rejection patterns cover narrow-purpose RPCs in Pioneer's
* catalog (Flashbots' rpc.flashbots.net is the canonical example — supports
* only eth_sendRawTransaction / eth_chainId / eth_blockNumber, rejects the
* rest with HTTP 403 + JSON-RPC -32601). Without them such URLs are sticky:
* their pre-flight getBlockNumber() passes so they get picked first, then
* every read fails. Treating the rejection as transient blacklists them for
* 60s and moves on.
*/
export const isTransientRpcError = (errMsg: string): boolean => {
const m = errMsg.toLowerCase();
return (
Expand All @@ -42,7 +64,24 @@ export const isTransientRpcError = (errMsg: string): boolean => {
m.includes('network') ||
m.includes('server_error') ||
m.includes('exceeded maximum retry') ||
/\b5\d{2}\b/.test(m) // 5xx
/\b5\d{2}\b/.test(m) || // 5xx
// Connection-level failure, browser wording.
m.includes('failed to fetch') || // Chrome / Edge
m.includes('load failed') || // Safari
m.includes('fetch failed') || // Node / undici
m.includes('err_') || // Chrome net errors: ERR_NAME_NOT_RESOLVED, ERR_CONNECTION_REFUSED, ...
m.includes('aborted') || // per-attempt AbortSignal.timeout fired
// Method-rejection: this URL doesn't support this method. Try next.
m.includes('rpc method is not whitelisted') ||
m.includes('method not found') ||
m.includes('method not supported') ||
m.includes('method does not exist') ||
m.includes('-32601') ||
// Narrow to ethers' transport-level wrapper text. A bare `.includes('403')`
// would misfire on revert reasons or hex payloads that happen to contain
// "403", replaying a successfully-rejected eth_call across every URL.
m.includes('server response 403') ||
m.includes('http 403')
);
};

Expand Down Expand Up @@ -115,7 +154,10 @@ export async function withRpcFailoverByNetworkId<T>(
} catch (e: any) {
const errMsg = String(e?.message || e);
if (!isTransientRpcError(errMsg)) {
// Definitive — won't help to try another RPC.
// Definitive — won't help to try another RPC. Log the URL: this
// branch used to throw silently, which made RPC failures look like
// they came from somewhere else entirely.
console.error(`[rpcFailover] ${networkId} ${url} definitive failure, aborting failover:`, errMsg);
throw e;
}
console.warn(`[rpcFailover] ${networkId} ${url} transient failure, trying next:`, errMsg);
Expand Down
2 changes: 1 addition & 1 deletion chrome-extension/src/background/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ chrome.runtime.onMessage.addListener((message: any, sender: any, sendResponse: a
`[HANDOFF] BEX → content script (${chain}/${method}) ERROR\n params=${JSON.stringify(params)}\n error=`,
error,
);
sendResponse({ error: formatUserError(error) });
sendResponse({ error: await formatUserError(error) });
}
} else {
sendResponse({ error: 'Invalid request: missing method' });
Expand Down
2 changes: 1 addition & 1 deletion chrome-extension/src/background/methods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ const routeWalletRequest = async (
}

// Translate "No device connected" SdkError into user-facing message
errorMessage = formatUserError({ message: errorMessage });
errorMessage = await formatUserError({ message: errorMessage });

//push error to the popup
// Forward `kind` so the side panel can render category-specific UI
Expand Down
42 changes: 30 additions & 12 deletions chrome-extension/src/background/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, it, expect } from 'vitest';
import { describe, it, expect, vi, afterEach } from 'vitest';
import {
createProviderRpcError,
createTimeoutError,
Expand Down Expand Up @@ -61,27 +61,45 @@ describe('isVaultUnreachableError', () => {
});

describe('formatUserError', () => {
it('translates a vault-unreachable network error into the launch instruction', () => {
expect(formatUserError(new Error('TypeError: Failed to fetch'))).toBe(VAULT_REQUIRED_MESSAGE);
// formatUserError probes localhost:1646 before blaming the vault, so every
// case here has to say whether the vault is up. `vaultUp(false)` = closed.
const vaultUp = (up: boolean) =>
vi.stubGlobal(
'fetch',
vi.fn(() => (up ? Promise.resolve(new Response('ok')) : Promise.reject(new TypeError('Failed to fetch')))),
);

afterEach(() => vi.unstubAllGlobals());

it('translates a vault-unreachable network error into the launch instruction', async () => {
vaultUp(false);
await expect(formatUserError(new Error('TypeError: Failed to fetch'))).resolves.toBe(VAULT_REQUIRED_MESSAGE);
});

// The regression this guards: Chrome throws the identical string for a dead
// Ethereum RPC, and users were told to launch a vault that was already up.
it('does NOT blame the vault when the vault answers', async () => {
vaultUp(true);
await expect(formatUserError(new Error('TypeError: Failed to fetch'))).resolves.toBe('TypeError: Failed to fetch');
});

it('translates the vault "No device connected" error into a friendly message', () => {
it('translates the vault "No device connected" error into a friendly message', async () => {
const e = new Error('SdkError: No device connected');
expect(formatUserError(e)).toBe('Please connect your KeepKey device and try again.');
await expect(formatUserError(e)).resolves.toBe('Please connect your KeepKey device and try again.');
});

it('passes other error messages through unchanged', () => {
expect(formatUserError(new Error('replacement transaction underpriced'))).toBe(
it('passes other error messages through unchanged', async () => {
await expect(formatUserError(new Error('replacement transaction underpriced'))).resolves.toBe(
'replacement transaction underpriced',
);
});

it('handles non-Error values by stringifying them', () => {
expect(formatUserError('plain string failure')).toBe('plain string failure');
it('handles non-Error values by stringifying them', async () => {
await expect(formatUserError('plain string failure')).resolves.toBe('plain string failure');
});

it('does not throw on null/undefined input', () => {
expect(() => formatUserError(null)).not.toThrow();
expect(() => formatUserError(undefined)).not.toThrow();
it('does not throw on null/undefined input', async () => {
await expect(formatUserError(null)).resolves.toBeDefined();
await expect(formatUserError(undefined)).resolves.toBeDefined();
});
});
31 changes: 25 additions & 6 deletions chrome-extension/src/background/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,43 @@ export const VAULT_REQUIRED_MESSAGE =
export const createVaultRequiredError = (): ProviderRpcError => createProviderRpcError(4900, VAULT_REQUIRED_MESSAGE);

/**
* True when an error came from the vault REST server being down. The signing
* path fetches localhost:1646; when the vault is closed that rejects with a
* network error whose message varies by browser/runtime ("Failed to fetch",
* "Load failed", "NetworkError", "ECONNREFUSED").
* True when an error *could* have come from the vault REST server being down.
* The signing path fetches localhost:1646; when the vault is closed that
* rejects with a network error whose message varies by browser/runtime
* ("Failed to fetch", "Load failed", "NetworkError", "ECONNREFUSED").
*
* Deliberately NOT sufficient on its own. Chrome throws the exact same
* "Failed to fetch" for a dead Ethereum RPC, so this test alone told users to
* launch a vault that was already running. `formatUserError` confirms with a
* live probe before claiming the vault is down — see `isVaultReachable`.
*/
export function isVaultUnreachableError(msg: string): boolean {
return /failed to fetch|load failed|networkerror|econnrefused|fetch failed|err_connection_refused/i.test(msg);
}

/**
* Ask the vault directly instead of guessing from error text. Cheap
* (localhost, only runs on an already-failed request) and authoritative,
* unlike the 5s-stale KEEPKEY_STATE poll. Same endpoint `checkKeepKey()` uses.
*/
export async function isVaultReachable(): Promise<boolean> {
try {
await fetch('http://localhost:1646/docs', { signal: AbortSignal.timeout(1500) });
return true;
} catch {
return false;
}
}

/**
* Translate low-level errors into user-facing messages:
* - vault unreachable (localhost:1646 down) → "Vault not running" instruction
* - vault SdkError ("No device connected") → connect-device instruction
* All other errors pass through unchanged.
*/
export function formatUserError(err: unknown): string {
export async function formatUserError(err: unknown): Promise<string> {
const msg = (err as Error)?.message ?? String(err);
if (isVaultUnreachableError(msg)) {
if (isVaultUnreachableError(msg) && !(await isVaultReachable())) {
return VAULT_REQUIRED_MESSAGE;
}
if (msg.includes('No device connected')) {
Expand Down
Loading