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

tests: Add exception error message for JSONRPCException #8810

Merged
merged 1 commit into from Sep 27, 2016

Conversation

@laanwj
Copy link
Member

commented Sep 25, 2016

This improves error reporting if JSONRPCException is not specifically caught and ends up in Python's default backtrace handler.

Before:

Traceback (most recent call last):
  File "/.../projects/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 153, in __call__
    raise JSONRPCException(response['error'])
test_framework.authproxy.JSONRPCException

After:

Traceback (most recent call last):
  File "/.../projects/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 152, in __call__
    raise JSONRPCException(response['error'])
test_framework.authproxy.JSONRPCException: Unknown named parameter random (-8)
tests: Add exception error message for JSONRPCException
This improves error reporting if `JSONRPCException` is not specifically caught
and ends up in Python's default backtrace handler.

Before:
```
Traceback (most recent call last):
  File "/.../projects/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 153, in __call__
    raise JSONRPCException(response['error'])
test_framework.authproxy.JSONRPCException
```

After:
```
Traceback (most recent call last):
  File "/.../projects/bitcoin/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 152, in __call__
    raise JSONRPCException(response['error'])
test_framework.authproxy.JSONRPCException: Unknown named parameter random (-8)
```

@laanwj laanwj added the Tests label Sep 25, 2016

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Sep 25, 2016

Could make sense to also cherry-pick to upstream?

@laanwj

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2016

Yes, would make sense in this case.

@laanwj

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2016

Looks like upstream already has similar functionality: https://github.com/jgarzik/python-bitcoinrpc/blob/master/bitcoinrpc/authproxy.py#L58
I like this solution better, though, as it also shows the error code.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Sep 27, 2016

You could cherry-pick upstream (jgarzik/python-bitcoinrpc#48 and modify it, so that the error code is passed as well...

Though, I don't feel strong about this. Unfortunately we already diverged from upstream quite a bit and it turns out for every problem in this file there are two comparable but different solutions. (One for Bitcoin Core and the other for the lib) So we are already "out of sync".

@laanwj

This comment has been minimized.

Copy link
Member Author

commented Sep 27, 2016

Yes I don't care deeply about this either. The projects have different goals, our copy is just for testing the version of Bitcoin Core in the repository while the upstream projects tries to keep as much compatibility as possible.

@laanwj laanwj merged commit 42f6aed into bitcoin:master Sep 27, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Sep 27, 2016
Merge #8810: tests: Add exception error message for JSONRPCException
42f6aed tests: Add exception error message for JSONRPCException (Wladimir J. van der Laan)
codablock added a commit to codablock/dash that referenced this pull request Jan 11, 2018
Merge bitcoin#8810: tests: Add exception error message for JSONRPCExc…
…eption

42f6aed tests: Add exception error message for JSONRPCException (Wladimir J. van der Laan)
andvgal added a commit to energicryptocurrency/energi that referenced this pull request Jan 6, 2019
Merge bitcoin#8810: tests: Add exception error message for JSONRPCExc…
…eption

42f6aed tests: Add exception error message for JSONRPCException (Wladimir J. van der Laan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.