fix(evm): fail over on Chrome network errors instead of claiming the vault is down - #134
Merged
Merged
Conversation
…vault is down
A dApp eth_sendTransaction failed with "KeepKey Vault is not running" while
the vault was running. Two message-text classifiers combined to turn a dead
Ethereum RPC into a false claim about the vault:
isTransientRpcError("Failed to fetch") -> false => failover aborts
isVaultUnreachableError("Failed to fetch") -> true => "Vault not running"
Chrome throws byte-identical text for an unreachable RPC and a closed vault,
so no regex can separate them.
- isTransientRpcError now covers browser connection-level wording
(failed to fetch / load failed / err_ / aborted). It was written against
Firefox/Node wording, so the same dead RPC failed over on Firefox and
hard-threw on Chrome. The loop now tries the remaining URLs.
- Collapse the two copies of the classifier (ethereumHandler + rpcFailover)
into one export; they had already drifted.
- Log the URL before both definitive throws. That branch was silent while the
transient branch logged, so the failing RPC never appeared in the console —
the single biggest reason this was misdiagnosed as a vault problem.
- formatUserError probes localhost:1646 before blaming the vault, instead of
inferring vault state from an arbitrary error string in a catch-all.
Tests: browser error strings in rpcFailover.test.ts; utils.test.ts asserts an
RPC-origin "Failed to fetch" does NOT produce VAULT_REQUIRED_MESSAGE when the
vault answers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
A dApp
eth_sendTransactionfailed with:The vault was running the whole time. Two message-text classifiers combine to turn an Ethereum RPC that won't connect into a false claim that the vault is offline:
Chrome throws byte-identical text for an unreachable RPC and a closed vault. No regex can separate them.
Concretely:
ethereum-rpc.publicnode.comis first inlastResortRpcs['eip155:1']and currently fails at TLS from some networks (curlexit 35).withRpcFailoverclassified that as definitive, aborted on the first URL, and never reachedeth.drpc.org— which works fine. The resulting error hit the catch-all inmethods.tsand got relabeled as a vault problem.Not a regression:
isTransientRpcErrorwas written against Firefox/Node wording (NetworkError when attempting to fetch resource), so the same dead RPC failed over on Firefox and hard-failed on Chrome. Latent since it was introduced; surfaced now because a first-in-list RPC started failing at connection level.Ruled out with evidence before touching anything: Pioneer discovery returns 8 mainnet RPCs in 0.13s (6 of 8 answer
eth_chainId), and the vault answers/api/healthhealthy with correct CORS including for achrome-extension://origin.Changes
isTransientRpcErrorlearns browser wording —failed to fetch,load failed,err_(ChromeERR_*net errors),aborted. The failover loop now tries the remaining URLs instead of dying on the first bad one.ethereumHandler.tshad its own copy with method-rejection patterns thatrpcFailover.tslacked; it now imports the shared classifier.formatUserErrorasks the vault instead of guessing. It now probeslocalhost:1646before claiming the vault is down. Cheap (localhost, only on an already-failed request) and authoritative, unlike inferring vault state from an arbitrary error string in a catch-all.Tests
128 passed, type-check and lint clean (0 errors).rpcFailover.test.ts— the browser connection-error strings, plus the method-rejection patterns that previously had no coverage in this module.utils.test.ts— an RPC-originFailed to fetchmust not produceVAULT_REQUIRED_MESSAGEwhen the vault answers. This is the regression guard.Verified manually
Built and loaded unpacked; dApp EVM send succeeds, console shows the failover from
publicnode.comtoeth.drpc.org. Vault-closed path re-checked separately and still shows the launch instruction.Deliberately not done
Reordering
lastResortRpcs['eip155:1']to puteth.drpc.orgfirst. One machine's TLS failure isn't grounds for changing a global default, and failover now covers it.🤖 Generated with Claude Code