Skip to content

Commit 9659ac3

Browse files
fix: Ensure non-object data in RPC errors is deserialized correctly (#7638)
## Explanation As-is the compatibility function `deserializeError` strips out valid `data` if it isn't an object. Serialized values based on the `JsonRpcError` class may have `data` that is any JSON value. This PR fixes the issue by adding an additional case to the deserialization to account for `data` being a non-object JSON value. This for example ensures that "execution reverted" errors from the node have the revert data properly populated. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Ensures `deserializeError` retains non-object JSON `data` for RPC errors while still merging `cause` appropriately. > > - Add `deserializeData` to merge object `data` with `cause` and pass through primitive/JSON `data` using `isValidJson` > - Update `deserializeError` to use `deserializeData`; import `OptionalDataWithOptionalCause` and `isValidJson` > - Extend tests to cover primitive `data` case and validated error message behavior > - Update `CHANGELOG.md` under Unreleased with the fix > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6d843cb. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 8257977 commit 9659ac3

File tree

3 files changed

+44
-5
lines changed

3 files changed

+44
-5
lines changed

packages/json-rpc-engine/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Upgrade `@metamask/utils` from `^11.8.1` to `^11.9.0` ([#7511](https://github.com/MetaMask/core/pull/7511))
1313

14+
### Fixed
15+
16+
- Ensure non-object data in RPC errors is deserialized correctly when using JsonRpcEngine compatibility tools ([#7638](https://github.com/MetaMask/core/pull/7638))
17+
1418
## [10.2.0]
1519

1620
### Added

packages/json-rpc-engine/src/v2/compatibility-utils.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,5 +525,17 @@ describe('compatibility-utils', () => {
525525

526526
expect(result.message).toBe('Internal JSON-RPC error.');
527527
});
528+
529+
it('uses correct data when data is a primitive', () => {
530+
const thrownValue = {
531+
code: 3,
532+
message: 'execution reverted',
533+
data: '0x556f1830000000000000000000000000de9049636f4a1dfe0a64d1bfe3155c0a14c54f3100000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000160f4d4d2f800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000028000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004d68747470733a2f2f6170692e636f696e626173652e636f6d2f6170692f76312f646f6d61696e2f7265736f6c7665722f7265736f6c7665446f6d61696e2f7b73656e6465727d2f7b646174617d0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e49061b92300000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000f046772656704626173650365746800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000243b3b57de068bcd633299a45145c9f0ec708cd91d078cede4afe492db21c009229d02a405000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e49061b92300000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000f046772656704626173650365746800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000243b3b57de068bcd633299a45145c9f0ec708cd91d078cede4afe492db21c009229d02a4050000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
534+
};
535+
536+
const result = deserializeError(thrownValue) as JsonRpcError<string>;
537+
538+
expect(result.data).toBe(thrownValue.data);
539+
});
528540
});
529541
});

packages/json-rpc-engine/src/v2/compatibility-utils.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
import type { OptionalDataWithOptionalCause } from '@metamask/rpc-errors';
12
import { getMessageFromCode, JsonRpcError } from '@metamask/rpc-errors';
23
import type { Json } from '@metamask/utils';
3-
import { hasProperty, isObject } from '@metamask/utils';
4+
import { hasProperty, isObject, isValidJson } from '@metamask/utils';
45
// ATTN: We must NOT use 'klona/full' here because it freezes properties on the clone.
56
import { klona } from 'klona';
67

@@ -132,6 +133,31 @@ export function propagateToRequest(
132133
});
133134
}
134135

136+
/**
137+
* Deserialize the error property for a thrown error, merging in the cause where possible.
138+
*
139+
* @param data - The data from the thrown error.
140+
* @param cause - The cause from the thrown error.
141+
* @returns The deserialized data.
142+
*/
143+
function deserializeData(
144+
data: unknown,
145+
cause: unknown,
146+
): OptionalDataWithOptionalCause {
147+
// If data is an object, merge with cause.
148+
if (isObject(data)) {
149+
return { ...data, cause };
150+
}
151+
152+
// If data is a JSON value that's not mergeable.
153+
if (isValidJson(data)) {
154+
return data;
155+
}
156+
157+
// If data is undefined, only use cause.
158+
return { cause };
159+
}
160+
135161
/**
136162
* Unserialize an error from a thrown value. Creates a {@link JsonRpcError} if
137163
* the thrown value is an object with a `code` property. Otherwise, creates a
@@ -177,10 +203,7 @@ export function deserializeError(thrown: unknown): Error | JsonRpcError<Json> {
177203
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
178204
// @ts-ignore - Our error type is outdated.
179205
new Error(message, { cause })
180-
: new JsonRpcError(code, message, {
181-
...(isObject(data) ? data : undefined),
182-
cause,
183-
});
206+
: new JsonRpcError(code, message, deserializeData(data, cause));
184207

185208
if (typeof stack === 'string') {
186209
error.stack = stack;

0 commit comments

Comments
 (0)