-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fix error deserialization bug #227
Conversation
Please explain the circumstances for when this happened. |
9f7c7a7
to
97a1ed4
Compare
71e48b7
to
2bdfd9c
Compare
2bdfd9c
to
74a3cfc
Compare
b300308
to
66227c4
Compare
We noticed in [1] that some servers return error response that has only a string error instead of an error object, so we're adding a test to prove the issue. The error originally happened when I passed uint256 as txId to GetBlockchainTransaction electrum request when it should've been a string. [1] nblockchain#221
66227c4
to
3ca8098
Compare
This commit fixes the deserialization error happening when returned error is a simple string instead of an object containing errorCode and errorMsg. The error originally happened when I passed uint256 as txId to GetBlockchainTransaction electrum request when it should've been a string. Co-authored-by: Mersho <[email protected]>
c0e307e
to
7d91cb9
Compare
I have the feeling that this PR has created a regression. I just got this running stable branch:
Please note: this didn't really crash the wallet, but it sends a WARNING to sentry because it is an unhandled exception that geewallet ultimately catches (with a catch-all), when doing FaultTolerantParallelClient queries. |
And now it crashed!:
|
Nevermind, I found the issue (it wasn't related to this PR). Just pushed a fix to stable branch. |
This PR fixes the deserialization error happening
when returned error is a simple string instead of an object
containing errorCode and errorMsg.