Skip to content

Update documentation on response errors#578

Merged
pisv merged 1 commit into
mainfrom
pisv-fix-576
Dec 12, 2021
Merged

Update documentation on response errors#578
pisv merged 1 commit into
mainfrom
pisv-fix-576

Conversation

@pisv

@pisv pisv commented Dec 5, 2021

Copy link
Copy Markdown
Contributor

Fixes #576

@pisv pisv merged commit f254f87 into main Dec 12, 2021
@pisv pisv deleted the pisv-fix-576 branch December 12, 2021 12:42
cdietrich pushed a commit that referenced this pull request Jan 13, 2022
@jonahgraham jonahgraham added this to the v0.13.0 milestone May 17, 2022
jonahgraham added a commit that referenced this pull request Feb 14, 2024
By wrapping ResponseErrorException with another layer in
RuntimeException it means the unwrapping of the exception in
RemoteEndpoint.DEFAULT_EXCEPTION_HANDLER doesn't work as
expected and instead of getting the original ResponseError
the other end gets the fallback ResponseError of type
internal error.

This change includes updates to documentation to show that
it is ok to throw ResponseErrorException (reverts #578).
While it is also OK to return an exceptionally completed
future, it is not needed to do that way and simply throwing
is more straightforward.

There are new tests to cover the changed handling. Also tested
is the previously documented way of handling exceptions (see
`tries == 1` case in `testVersatility` and `testVersatilityResponseError`

Fixes #802
jonahgraham added a commit that referenced this pull request Feb 20, 2024
By wrapping ResponseErrorException with another layer in
RuntimeException it means the unwrapping of the exception in
RemoteEndpoint.DEFAULT_EXCEPTION_HANDLER doesn't work as
expected and instead of getting the original ResponseError
the other end gets the fallback ResponseError of type
internal error.

This change includes updates to documentation to show that
it is ok to throw ResponseErrorException (reverts #578).
While it is also OK to return an exceptionally completed
future, it is not needed to do that way and simply throwing
is more straightforward.

There are new tests to cover the changed handling. Also tested
is the previously documented way of handling exceptions (see
`tries == 1` case in `testVersatility` and `testVersatilityResponseError`

Fixes #802

Also-by: Vladimir Piskarev <pisv@1c.ru>
jonahgraham added a commit that referenced this pull request Feb 29, 2024
By wrapping ResponseErrorException with another layer in
RuntimeException it means the unwrapping of the exception in
RemoteEndpoint.DEFAULT_EXCEPTION_HANDLER doesn't work as
expected and instead of getting the original ResponseError
the other end gets the fallback ResponseError of type
internal error.

This change includes updates to documentation to show that
it is ok to throw ResponseErrorException (reverts #578).
While it is also OK to return an exceptionally completed
future, it is not needed to do that way and simply throwing
is more straightforward.

There are new tests to cover the changed handling. Also tested
is the previously documented way of handling exceptions (see
`tries == 1` case in `testVersatility` and `testVersatilityResponseError`

Fixes #802

Also-by: Vladimir Piskarev <pisv@1c.ru>
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.

JSON-RPC Exceptions are not propagated correctly with the default exception handler

2 participants