From 8f6d9613d9e9d2cd7a10ea2f03dfefe9e81a019f Mon Sep 17 00:00:00 2001 From: xvlad <116202536+sktbrd@users.noreply.github.com> Date: Fri, 31 Jul 2026 17:32:01 -0300 Subject: [PATCH] fix(injected): reject with an Error, not a bare string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #132. The background sends failures across postMessage as a plain string (`sendResponse({ error: formatUserError(error) })`), and every injected provider rejected with that string untouched. EIP-1193 requires rejecting with an object carrying `code` and `message`, and dApp libraries rely on it: wagmi/viem/ethers inspect the rejection value, `in` throws on a primitive, and the user gets TypeError: Cannot use 'in' operator to search for 'data' in KeepKey Vault is not running. Open the KeepKey Vault desktop app... instead of the instruction we wrote for exactly that situation. The message we most want read is the one we made unreadable. Nothing about that message is special — every error from the extension rejected as a primitive. It is just long enough to make the TypeError absurd. Add toProviderError() and apply it at all five reject sites: injected.ts (EVM), solana-provider, tron-provider, solana-wallet-standard. It keeps an existing Error by reference so stacks are not discarded, preserves a `code` when one is already set, rebuilds structured errors that lost their prototype crossing postMessage, and never throws — a normalizer that can fail is worse than the bug it fixes. Follow-up (deliberately not in this change): formatUserError() flattens errors to a string, so the 4900 "provider disconnected" code from createVaultRequiredError() is dropped before it reaches the dApp. Sending `{ message, code }` would preserve it — toProviderError already handles that shape — but it changes what every consumer of `response.error` receives and wants a side-panel audit first. Co-Authored-By: Claude Opus 5 --- chrome-extension/src/injected/injected.ts | 3 +- .../src/injected/provider-error.test.ts | 56 ++++++++++++++++ .../src/injected/provider-error.ts | 67 +++++++++++++++++++ .../src/injected/solana-provider.ts | 3 +- .../src/injected/solana-wallet-standard.ts | 3 +- .../src/injected/tron-provider.ts | 3 +- 6 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 chrome-extension/src/injected/provider-error.test.ts create mode 100644 chrome-extension/src/injected/provider-error.ts diff --git a/chrome-extension/src/injected/injected.ts b/chrome-extension/src/injected/injected.ts index b13d267..3c7da25 100644 --- a/chrome-extension/src/injected/injected.ts +++ b/chrome-extension/src/injected/injected.ts @@ -13,6 +13,7 @@ import { registerSolanaWallet } from './solana-wallet-register'; import { KeepKeySolanaProvider } from './solana-provider'; import { KeepKeyTronProvider } from './tron-provider'; import { createHiveKeychainShim } from './hive-provider'; +import { toProviderError } from './provider-error'; import { installConsoleCapture } from './consoleCapture'; import { installPageObserver } from './pageObserver'; @@ -357,7 +358,7 @@ import { installPageObserver } from './pageObserver'; `[HANDOFF] dApp ← KeepKey (${chain}/${method}) REJECT\n params=${JSON.stringify(params)}\n error=`, error, ); - reject(error); + reject(toProviderError(error, `${method} failed`)); } else { const resultType = typeof result; const resultPreview = diff --git a/chrome-extension/src/injected/provider-error.test.ts b/chrome-extension/src/injected/provider-error.test.ts new file mode 100644 index 0000000..7b73d06 --- /dev/null +++ b/chrome-extension/src/injected/provider-error.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect } from 'vitest'; +import { toProviderError } from './provider-error'; + +const VAULT_MESSAGE = + 'KeepKey Vault is not running. Open the KeepKey Vault desktop app, then try again. Get it at https://keepkey.com/launch'; + +describe('toProviderError', () => { + it('wraps a bare string so dApp libraries can inspect it', () => { + // The regression: the background sends failures as a plain string, and + // rejecting with a primitive made wagmi/viem/ethers throw + // "Cannot use 'in' operator to search for 'data' in ", + // hiding the actual instruction from the user. + const err = toProviderError(VAULT_MESSAGE); + + expect(err).toBeInstanceOf(Error); + expect(err.message).toBe(VAULT_MESSAGE); + expect(err.code).toBe(-32603); + expect(() => 'data' in err).not.toThrow(); + }); + + it('preserves an existing Error, adding a code when missing', () => { + const original = new Error('boom'); + const err = toProviderError(original); + + expect(err).toBe(original); // same reference — stack is not discarded + expect(err.code).toBe(-32603); + }); + + it('does not clobber a code the caller already set', () => { + const original = Object.assign(new Error('disconnected'), { code: 4900 }); + expect(toProviderError(original).code).toBe(4900); + }); + + it('rebuilds a structured error that lost its prototype over postMessage', () => { + const err = toProviderError({ message: 'user rejected', code: 4001, data: { hint: 'x' } }); + + expect(err).toBeInstanceOf(Error); + expect(err.message).toBe('user rejected'); + expect(err.code).toBe(4001); + expect(err.data).toEqual({ hint: 'x' }); + }); + + it('falls back for null, undefined and empty values', () => { + expect(toProviderError(null, 'eth_call failed').message).toBe('eth_call failed'); + expect(toProviderError(undefined, 'eth_call failed').message).toBe('eth_call failed'); + expect(toProviderError('', 'eth_call failed').message).toBe('eth_call failed'); + expect(toProviderError({}, 'eth_call failed').message).toBe('eth_call failed'); + }); + + it('never throws, whatever it is handed', () => { + for (const input of [0, false, Symbol('s'), 123n, [], () => {}]) { + expect(() => toProviderError(input)).not.toThrow(); + expect(toProviderError(input)).toBeInstanceOf(Error); + } + }); +}); diff --git a/chrome-extension/src/injected/provider-error.ts b/chrome-extension/src/injected/provider-error.ts new file mode 100644 index 0000000..f625e31 --- /dev/null +++ b/chrome-extension/src/injected/provider-error.ts @@ -0,0 +1,67 @@ +/** + * Error normalization for the injected providers. + * + * EIP-1193 requires a provider to reject with an object carrying `code` and + * `message`. Our background script sends failures across postMessage as a bare + * string (`sendResponse({ error: formatUserError(error) })`), and every + * provider used to `reject(error)` with that string untouched. + * + * Rejecting with a primitive breaks the dApp libraries downstream, because + * they inspect the rejection value before showing it. wagmi/viem/ethers all do + * some form of membership test, and `in` throws on a primitive: + * + * TypeError: Cannot use 'in' operator to search for 'data' in + * KeepKey Vault is not running. Open the KeepKey Vault desktop app... + * + * The user then sees a JavaScript error instead of the instruction we went to + * the trouble of writing. Normalizing here — at the boundary where values + * leave us for dApp code — fixes every provider at once and keeps us honest + * with the spec regardless of what the background sends. + */ + +/** EIP-1193 provider error: an Error with a numeric `code`, optionally `data`. */ +export interface ProviderRpcError extends Error { + code: number; + data?: unknown; +} + +/** JSON-RPC internal error — the safe default when no code survived the trip. */ +const INTERNAL_ERROR = -32603; + +/** + * Coerce anything a provider might be handed into a proper ProviderRpcError. + * + * Preserves an existing `code`/`data` when present, so a meaningful code such + * as 4900 ("provider disconnected") still reaches the dApp. Never throws — a + * normalizer that can fail is worse than the bug it fixes. + */ +export function toProviderError(raw: unknown, fallbackMessage = 'Request failed'): ProviderRpcError { + // Already an Error: attach a code if it lacks one and pass it through, so we + // don't discard a stack or a subclass the caller cared about. + if (raw instanceof Error) { + const err = raw as ProviderRpcError; + if (typeof err.code !== 'number') err.code = INTERNAL_ERROR; + return err; + } + + if (typeof raw === 'string') { + const err = new Error(raw || fallbackMessage) as ProviderRpcError; + err.code = INTERNAL_ERROR; + return err; + } + + // Structured error that lost its prototype crossing postMessage, e.g. + // { message, code, data }. Keep whatever fields made it across. + if (raw && typeof raw === 'object') { + const src = raw as { message?: unknown; code?: unknown; data?: unknown }; + const message = typeof src.message === 'string' && src.message ? src.message : fallbackMessage; + const err = new Error(message) as ProviderRpcError; + err.code = typeof src.code === 'number' ? src.code : INTERNAL_ERROR; + if (src.data !== undefined) err.data = src.data; + return err; + } + + const err = new Error(fallbackMessage) as ProviderRpcError; + err.code = INTERNAL_ERROR; + return err; +} diff --git a/chrome-extension/src/injected/solana-provider.ts b/chrome-extension/src/injected/solana-provider.ts index 99ec4ba..81ed839 100644 --- a/chrome-extension/src/injected/solana-provider.ts +++ b/chrome-extension/src/injected/solana-provider.ts @@ -25,6 +25,7 @@ */ import type { ChainType } from './types'; +import { toProviderError } from './provider-error'; type WalletRequestFn = ( method: string, @@ -335,7 +336,7 @@ export class KeepKeySolanaProvider { #rpc(method: string, params: any[]): Promise { return new Promise((resolve, reject) => { this.#walletRequest(method, params, 'solana' as ChainType, (error, result) => { - if (error) reject(error); + if (error) reject(toProviderError(error, `${method} failed`)); else resolve(result); }); }); diff --git a/chrome-extension/src/injected/solana-wallet-standard.ts b/chrome-extension/src/injected/solana-wallet-standard.ts index b39713d..1edbd61 100644 --- a/chrome-extension/src/injected/solana-wallet-standard.ts +++ b/chrome-extension/src/injected/solana-wallet-standard.ts @@ -7,6 +7,7 @@ */ import type { ChainType } from './types'; +import { toProviderError } from './provider-error'; // ---------- Base58 (inline, no external dep) ---------- @@ -374,7 +375,7 @@ export class KeepKeySolanaWallet { #rpc(method: string, params: any[]): Promise { return new Promise((resolve, reject) => { this.#walletRequest(method, params, 'solana' as ChainType, (error, result) => { - if (error) reject(error); + if (error) reject(toProviderError(error, `${method} failed`)); else resolve(result); }); }); diff --git a/chrome-extension/src/injected/tron-provider.ts b/chrome-extension/src/injected/tron-provider.ts index acafe01..eebe964 100644 --- a/chrome-extension/src/injected/tron-provider.ts +++ b/chrome-extension/src/injected/tron-provider.ts @@ -30,6 +30,7 @@ */ import type { ChainType } from './types'; +import { toProviderError } from './provider-error'; type WalletRequestFn = ( method: string, @@ -122,7 +123,7 @@ class EventEmitter { function promisifyRequest(walletRequest: WalletRequestFn, method: string, params: any[]): Promise { return new Promise((resolve, reject) => { walletRequest(method, params, 'tron', (error, result) => { - if (error) reject(error); + if (error) reject(toProviderError(error, `${method} failed`)); else resolve(result); }); });