Skip to content

fix(proxyd): Fix compliance with JSON-RPC 2.0 spec by adding optional RPCError.Data#3683

Merged
mslipper merged 3 commits intoethereum-optimism:developfrom
chriswessels:proxyd/fix-error-spec-conformance
Oct 11, 2022
Merged

fix(proxyd): Fix compliance with JSON-RPC 2.0 spec by adding optional RPCError.Data#3683
mslipper merged 3 commits intoethereum-optimism:developfrom
chriswessels:proxyd/fix-error-spec-conformance

Conversation

@chriswessels
Copy link
Contributor

@chriswessels chriswessels commented Oct 10, 2022

Description

The JSON-RPC 2.0 specification indicates that the error object may contain an optional data field. Proxyd currently drops this field from any responses. This PR fixes that.

https://www.jsonrpc.org/specification#error_object

Tests

I added a test to check that if a data field is present in the response from a backend, it will be added to the response from proxyd. The existing test should validate that if data field is not present in the response from the backend, the data key should not be present in the JSON response from proxyd.

@changeset-bot
Copy link

changeset-bot bot commented Oct 10, 2022

🦋 Changeset detected

Latest commit: bb7502f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/proxyd Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mergify
Copy link
Contributor

mergify bot commented Oct 10, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

Copy link
Contributor

@mslipper mslipper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, thank you!

@mslipper mslipper merged commit b3c5eee into ethereum-optimism:develop Oct 11, 2022
This was referenced Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants