eclipse-lsp4j
Unwrap InvocationTargetException which are ResponseErrorException
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 https://github.com/eclipse-lsp4j/lsp4j/pull/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 https://github.com/eclipse-lsp4j/lsp4j/issues/802
I updated docs and updated tests. New commit was used to update description of the PR
There is one todo thing left, hence turned to draft, the code change when it is delegate segments has no test coverage. Probably also worth adding to changelog.
There is one todo thing left, hence turned to draft, the code change when it is delegate segments has no test coverage. Probably also worth adding to changelog.
Newest commit adds to the changelog. It also adds the code coverage as the find delegate's change I had done previously was incorrect.
The 'sneaky throws' change is ready for review: https://github.com/eclipse-lsp4j/lsp4j/pull/809/commits/4de67343c5a30bf9110ace2969c832708e06e1df :-)
I have another approach in the works, which does not use sneaky throws. It is based on the fact that an Endpoint is not expected to throw a checked exception. Therefore, throwing a checked exception from an annotated jsonrpc method can (and should) be treated as a programming error, and can be signalled via an IllegalStateException in the GenericEndpoint. An inaccessible jsonrpc method (IllegalAccessException) can also be handled in a similar way. There is no need to use sneaky throws or a CompletionException in this case. Runtime exceptions or errors would just be re-thrown.
The latest change is ready for review: https://github.com/eclipse-lsp4j/lsp4j/pull/809/commits/3982fd3ba22c2ae71cebafdc3647dbfd347ff770.
@ivy-cst this change should end up on snapshot builds soon: https://github.com/eclipse-lsp4j/lsp4j?tab=readme-ov-file#snapshots
Please comment in #807 if you have input on if/when a full release will help your adoption so that I can consider how and when to make a release.
@ivy-cst this change should end up on snapshot builds soon: https://github.com/eclipse-lsp4j/lsp4j?tab=readme-ov-file#snapshots
Please comment in #807 if you have input on if/when a full release will help your adoption so that I can consider how and when to make a release.
Thank you for the fix. We do not need a release. We are currently living with our own exception handler. We are tied to the eclipse rcp releases at the moment anyway.
OK thanks for responding - if that changes please comment on #807
PS I really appreciate that you followed up on this issue, especially knowing that it did not affect your code in the short term. It will now make everyone's code in the future better and is part of what makes me very happy to be working on open source projects with a vibrant community!