Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue 549] Update JSON-RPC over HTTP error responses to return 200 status code #1426

Merged
merged 16 commits into from
Oct 8, 2020

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Oct 7, 2020

PR description

  • Updated returned HTTP status code to 200 for JSON-RPC error
  • Updated tests related to the modified HTTP status code

Fixed Issue(s)

Was #1305, Fixes #549

Changelog

  • I thought about the changelog and included a changelog update if required.

gabrieledm and others added 10 commits August 13, 2020 11:02
… JsonRpcError

Signed-off-by: gabrieledm <gabriele.delmonte90@gmail.com>
…tus code

Signed-off-by: gabrieledm <gabriele.delmonte90@gmail.com>
Signed-off-by: gabrieledm <gabriele.delmonte90@gmail.com>
Signed-off-by: gabrieledm <gabriele.delmonte90@gmail.com>
Signed-off-by: gabrieledm <gabriele.delmonte90@gmail.com>
The Web3J library is out of conformance for how JSON-RPC is supposed to
behave.  Since this PR is to bring Besu into conformance this breakage
is unfortunately expected.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Copy link
Contributor Author

shemnon commented Oct 7, 2020

This is a re-spin of #1305, with the disabling of some acceptance tests added in. Re-spun to ensure we can fast-track it into the RC1 build on Thursday.

@gabrieledm did all the interesting work.

@shemnon
Copy link
Contributor Author

shemnon commented Oct 7, 2020

Also, see issue #1425 reminding us to fix the Acceptance Tests.

@shemnon shemnon changed the title [Issue 549] Update JSON-RPC over HTTP error responses to return 200 status code #1305 [Issue 549] Update JSON-RPC over HTTP error responses to return 200 status code Oct 7, 2020
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM - just needs a changelog entry :)

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

couple of rewording suggestions

CHANGELOG.md Outdated

Prior versions of Besu would set the HTTP Status 400 Bad Request for JSON-RPC requests that completed in an error, regardless of the kind of error. These responses could include a complete JSON-RPC response with an error field.

In Besu version 20.10 properly formatted requests that have valid parameters (count and content) that otherwise would result in an error will return a HTTP Status 200 OK a JSON-RPC response with an error field will be returned. For example, requesting an account that does not exist in the chain or a block by hash that Besu does not have will now return HTTP 200 OK responses. Unparsable requests, improperly formatted requests, or requests with invalid parameters will continue to return HTTP 400 Bad Request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: In Besu version 20.10, properly formatted requests that have valid parameters (count and content) will return a HTTP Status 200 OK, with an error field if an error occurred. For example, requesting an account that does not exist in the chain, or a block by hash that Besu does not have, will now return HTTP 200 OK responses. Unparsable requests, improperly formatted requests, or requests with invalid parameters will continue to return HTTP 400 Bad Request.

CHANGELOG.md Outdated

In Besu version 20.10 properly formatted requests that have valid parameters (count and content) that otherwise would result in an error will return a HTTP Status 200 OK a JSON-RPC response with an error field will be returned. For example, requesting an account that does not exist in the chain or a block by hash that Besu does not have will now return HTTP 200 OK responses. Unparsable requests, improperly formatted requests, or requests with invalid parameters will continue to return HTTP 400 Bad Request.

This was done to bring us more in line with the behavior of other Ethereum Clients. Some community project, such as Web3J versions prior to 4.6.x, will be providing compatible releases in the near future.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: This was done to bring us more in line with the behavior of other Ethereum Clients. Some community projects, such as Web3J, will be providing compatible releases in the near future.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
…dm/master

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon shemnon merged commit 3ba84cc into hyperledger:master Oct 8, 2020
@shemnon shemnon deleted the gabrieledm/master branch October 8, 2020 04:28
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.

Update JSON-RPC over HTTP error responses to return 200 status code
3 participants