Conversation
wenfix
changed the title
# Conflicts: # packages/connect-multichain/CHANGELOG.md
| mockCore.invokeMethod.mockRejectedValue( | ||
| new RPCInvokeMethodErr( | ||
| 'RPC Request failed with code 4001: User denied transaction signature.', | ||
| 'User rejected the request', |
There was a problem hiding this comment.
Good catch — aligned the fixture to the canonical 'User rejected the request.' (with the trailing period) in 685619e.
| ); | ||
| const ancestorMessage = getFirstNonEmptyMessage([ | ||
| primitiveMessage, | ||
| ...ancestorObjects.map((object) => object.message), |
There was a problem hiding this comment.
i'm not certain it makes sense to try to resolve an error message from earlier up in the chain. Hard for me to fully understand this problem though. What were your thoughts around this?
There was a problem hiding this comment.
Good instinct, you're right. The rpc fields were sourced from anywhere in the cause chain, so a transport/API wrapper message could leak into rpcMessage and pose as the wallet's protocol message. Fixed in 1527a1e: rpcCode/rpcMessage/rpcData now read only from the coded wallet node; reason still falls back through the chain for readable logs. When the wallet sends no message, rpcMessage is left unset.
| * @param error - Unknown error thrown or returned during method execution. | ||
| * @returns Canonical fields for RPCInvokeMethodErr. | ||
| */ | ||
| function getInvocationErrorDetails(error: unknown): InvocationErrorDetails { |
There was a problem hiding this comment.
thoughts on moving all these newly added error helpers into their own file?
There was a problem hiding this comment.
quite a bit of complexity in this helper. How deeply nested are we expecting causes to be?
There was a problem hiding this comment.
Agreed — moved them into invocationError.ts (exporting toRPCInvokeMethodErr) in b4b3448, so the router stays focused and the normalization logic is testable in isolation.
There was a problem hiding this comment.
Shallow in practice (wallet → transport → maybe an API wrapper, ~2–3 levels); the depth-5 cap is just a guard against cyclic chains, not an expected depth. Most of the complexity was the message juggling feeding rpcMessage — that's gone now that the rpc fields read only from the coded node (1527a1e), so the remaining walk is just "find the first node with a numeric code."
|
The Claude traced it end to end. The wallet does send it:
Since both reject (rather than resolve an error envelope), the new If we want the changelog claim to hold, the fix would need to live in the transports' EDIT: pushed what I + claude think we need here: 432d04c |
DefaultTransport and MWPTransport dropped the JSON-RPC `error.data` while constructing the rejected error, so revert reasons / custom-error bytes never reached RequestRouter and RPCInvokeMethodErr.rpcData was always unset. Both transports now carry `data` (validated via isValidJson), so it survives to the router and reaches dapps via `error.data`. Adds a regression test per transport. Also drops the now-redundant rpcData cast in EIP1193Provider.request().
3 participants
Explanation
connect-multichain.invokeMethod()could surface different error shapes depending on whether a wallet error arrived as a resolved JSON-RPC error response or as a rejected transport error. That also meantconnect-evmcould lose provider-facing details when adapting multichain errors to EIP-1193 request errors.This change normalizes wallet invocation failures at the
RequestRouterboundary:code,message, and JSON-RPCdatafrom response errors and wrapped transport errorscausechain so transport/API wrappers do not hide the wallet errorRPCInvokeMethodErrwhile preservingreason,rpcCode,rpcMessage, andrpcDataRPCInvokeMethodErrinstances unchangedconnect-evmnow maps normalizedRPCInvokeMethodErrinstances into EIP-1193-facing errors that carry the walletcode, provider-facingmessage, and JSON-RPCdatawhen present.References
Checklist