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

eth, les: Consider context errors in vmError function #19196

Closed
wants to merge 1 commit into from

Conversation

AusIV
Copy link
Contributor

@AusIV AusIV commented Mar 1, 2019

In an eth_call request, if the EVM processing takes > 5 seconds
the request gets cancelled, but this is not resulting in an error
to the eth_call request (instead it responds with a result of "0x"
regardless of the called function).

With this in place, when a request times out, it will return an error
of:

"error":{"code":-32000,"message":"context deadline exceeded"}

Rather than incorrectly returning a null value.

In an `eth_call` request, if the EVM processing takes > 5 seconds
the request gets cancelled, but this is not resulting in an error
to the `eth_call` request (instead it responds with a result of "0x"
regardless of the called function).

With this in place, when a request times out, it will return an error
of:

`"error":{"code":-32000,"message":"context deadline exceeded"}`

Rather than incorrectly returning a null value.
@AusIV
Copy link
Contributor Author

AusIV commented Mar 1, 2019

I'm not certain due to the lack of detail in the other issue, but I believe this is will solve #19186

@AusIV
Copy link
Contributor Author

AusIV commented Mar 1, 2019

It looks like the appveyor build failure is also failing on master. I'm pretty sure it's not related to my changes - if you think it is let me know and I'll look into it.

@rjl493456442
Copy link
Member

Have you ever tried this PR #19737?
The issue you mentioned should be fixed.

@karalabe
Copy link
Member

This seems to have already been fixed upstream. Please check and if not, we can reopen it.

@karalabe karalabe closed this Nov 19, 2019
@adamschmideg
Copy link
Contributor

Presumably fixed by #19737 If not, please reopen this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants