Skip to content

Commit

Permalink
Unwrap InvocationTargetException which are ResponseErrorException
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jonahgraham
jonahgraham committed Feb 14, 2024
1 parent 88868d3 commit 4c49a57
Show file tree
Hide file tree
Showing 4 changed files with 191 additions and 8 deletions.
8 changes: 3 additions & 5 deletions documentation/jsonrpc.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,16 @@ For the other direction, if the implementation calls request on the RemoteEndpoi

The receiver of a request always needs to return a response message to conform to the JSON-RPC specification. In case the result value cannot be provided in a response because of an error, the `error` property of the `ResponseMessage` must be set to a `ResponseError` describing the failure.

This can be done by returning a `CompletableFuture` completed exceptionally with a `ResponseErrorException` from the request message handler in a local endpoint. The exception carries a `ResponseError` to attach to the response. The `RemoteEndpoint` will handle the exceptionally completed future and send a response message with the attached error object.
This can be done by throwing a `ResponseErrorException` from the request message handler in a local endpoint. The exception carries a `ResponseError` to attach to the response. The `RemoteEndpoint` will handle the exception and send a response message with the attached error object.

For example:

```java
@Override
public CompletableFuture<Object> shutdown() {
if (!isInitialized()) {
CompletableFuture<Object> exceptionalResult = new CompletableFuture<>();
ResponseError error = new ResponseError(ResponseErrorCode.ServerNotInitialized, "Server was not initialized", null);
exceptionalResult.completeExceptionally(new ResponseErrorException(error));
return exceptionalResult;
ResponseError error = new ResponseError(ResponseErrorCode.serverNotInitialized, "Server was not initialized", null);
throw new ResponseErrorException(error);
}
return doShutdown();
}
Expand Down
16 changes: 14 additions & 2 deletions ...lipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/services/GenericEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,13 @@ protected void recursiveFindRpcMethods(Object current, Set<Class<?>> visited, Se
Method method = methodInfo.method;
Object[] arguments = this.getArguments(method, arg);
return (CompletableFuture<Object>) method.invoke(current, arguments);
} catch (InvocationTargetException | IllegalAccessException e) {
} catch (InvocationTargetException e) {
if (e.getTargetException() instanceof ResponseErrorException) {
ResponseErrorException ree = (ResponseErrorException) e.getTargetException();
throw ree;
}
throw new RuntimeException(e);
} catch ( IllegalAccessException e) {
throw new RuntimeException(e);
}
};
Expand All @@ -79,7 +85,13 @@ protected void recursiveFindRpcMethods(Object current, Set<Class<?>> visited, Se
} else {
LOG.fine("A delegate object is null, jsonrpc methods of '" + method + "' are ignored");
}
} catch (InvocationTargetException | IllegalAccessException e) {
} catch (InvocationTargetException e) {
if (e.getTargetException() instanceof ResponseErrorException) {
ResponseErrorException ree = (ResponseErrorException) e.getTargetException();
throw ree;
}
throw new RuntimeException(e);
} catch ( IllegalAccessException e) {
throw new RuntimeException(e);
}
});
Expand Down
106 changes: 105 additions & 1 deletion org.eclipse.lsp4j.jsonrpc/src/test/java/org/eclipse/lsp4j/jsonrpc/test/IntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;
import org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.eclipse.lsp4j.jsonrpc.messages.ResponseError;
import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode;
import org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint;
import org.eclipse.lsp4j.jsonrpc.services.JsonNotification;
import org.eclipse.lsp4j.jsonrpc.services.JsonRequest;
Expand Down Expand Up @@ -420,8 +422,14 @@ public CompletableFuture<MyParam> askClient(MyParam param) {
tries++;
throw new UnsupportedOperationException();
}
if (tries == 1) {
tries++;
CompletableFuture<MyParam> future = new CompletableFuture<>();
future.completeExceptionally(new UnsupportedOperationException());
return future;
}
return CompletableFutures.computeAsync(executor, cancelToken -> {
if (tries++ == 1)
if (tries++ == 2)
throw new UnsupportedOperationException();
return param;
});
Expand All @@ -436,20 +444,116 @@ public CompletableFuture<MyParam> askClient(MyParam param) {
clientSideLauncher.startListening();
serverSideLauncher.startListening();

// tries == 0
CompletableFuture<MyParam> errorFuture1 = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
try {
System.out.println(errorFuture1.get());
Assert.fail();
} catch (ExecutionException e) {
Assert.assertNotNull(((ResponseErrorException)e.getCause()).getResponseError().getMessage());
}

// tries == 1
CompletableFuture<MyParam> errorFuture2 = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
try {
errorFuture2.get();
Assert.fail();
} catch (ExecutionException e) {
Assert.assertNotNull(((ResponseErrorException)e.getCause()).getResponseError().getMessage());
}

// tries == 2
CompletableFuture<MyParam> errorFuture3 = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
try {
errorFuture3.get();
Assert.fail();
} catch (ExecutionException e) {
Assert.assertNotNull(((ResponseErrorException)e.getCause()).getResponseError().getMessage());
}

// tries == 3
CompletableFuture<MyParam> goodFuture = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
Assert.assertEquals("FOO", goodFuture.get(TIMEOUT, TimeUnit.MILLISECONDS).value);
}

@Test
public void testVersatilityResponseError() throws Exception {
Logger.getLogger(RemoteEndpoint.class.getName()).setLevel(Level.OFF);
// create client side
PipedInputStream in = new PipedInputStream();
PipedOutputStream out = new PipedOutputStream();
PipedInputStream in2 = new PipedInputStream();
PipedOutputStream out2 = new PipedOutputStream();

// See https://github.com/eclipse-lsp4j/lsp4j/issues/510 for full details.
// Make sure that the thread that writes to the PipedOutputStream stays alive
// until the read from the PipedInputStream. Using a cached thread pool
// does not 100% guarantee that, but increases the probability that the
// selected thread will exist for the lifetime of the test.
ExecutorService executor = Executors.newCachedThreadPool();

in.connect(out2);
out.connect(in2);

MyClient client = new MyClient() {
private int tries = 0;

@Override
public CompletableFuture<MyParam> askClient(MyParam param) {
if (tries == 0) {
tries++;
throw new ResponseErrorException(new ResponseError(ResponseErrorCode.InvalidParams, "Direct Throw", "data"));
}
if (tries == 1) {
tries++;
CompletableFuture<MyParam> future = new CompletableFuture<>();
future.completeExceptionally(new ResponseErrorException(new ResponseError(ResponseErrorCode.InvalidParams, "completeExceptionally", "data")));
return future;
}
return CompletableFutures.computeAsync(executor, cancelToken -> {
if (tries++ == 2)
throw new ResponseErrorException(new ResponseError(ResponseErrorCode.InvalidParams, "Throw in computeAsync", "data"));
return param;
});
}
};
Launcher<MyServer> clientSideLauncher = Launcher.createLauncher(client, MyServer.class, in, out);

// create server side
MyServer server = new MyServerImpl();
Launcher<MyClient> serverSideLauncher = Launcher.createLauncher(server, MyClient.class, in2, out2);

clientSideLauncher.startListening();
serverSideLauncher.startListening();

// tries == 0
CompletableFuture<MyParam> errorFuture1 = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
try {
System.out.println(errorFuture1.get());
Assert.fail();
} catch (ExecutionException e) {
Assert.assertEquals("Direct Throw", ((ResponseErrorException)e.getCause()).getResponseError().getMessage());
}

// tries == 1
CompletableFuture<MyParam> errorFuture2 = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
try {
errorFuture2.get();
Assert.fail();
} catch (ExecutionException e) {
Assert.assertEquals("completeExceptionally", ((ResponseErrorException)e.getCause()).getResponseError().getMessage());
}

// tries == 2
CompletableFuture<MyParam> errorFuture3 = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
try {
errorFuture3.get();
Assert.fail();
} catch (ExecutionException e) {
Assert.assertEquals("Throw in computeAsync", ((ResponseErrorException)e.getCause()).getResponseError().getMessage());
}

// tries == 3
CompletableFuture<MyParam> goodFuture = serverSideLauncher.getRemoteProxy().askClient(new MyParam("FOO"));
Assert.assertEquals("FOO", goodFuture.get(TIMEOUT, TimeUnit.MILLISECONDS).value);
}
Expand Down
69 changes: 69 additions & 0 deletions ...clipse.lsp4j.jsonrpc/src/test/java/org/eclipse/lsp4j/jsonrpc/test/RemoteEndpointTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.eclipse.lsp4j.jsonrpc.JsonRpcException;
import org.eclipse.lsp4j.jsonrpc.MessageConsumer;
import org.eclipse.lsp4j.jsonrpc.RemoteEndpoint;
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.eclipse.lsp4j.jsonrpc.messages.Message;
import org.eclipse.lsp4j.jsonrpc.messages.MessageIssue;
Expand All @@ -42,6 +43,7 @@
import org.eclipse.lsp4j.jsonrpc.messages.ResponseError;
import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode;
import org.eclipse.lsp4j.jsonrpc.messages.ResponseMessage;
import org.eclipse.lsp4j.jsonrpc.test.IntegrationTest.MyParam;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -219,6 +221,73 @@ public CompletableFuture<Object> request(String method, Object parameter) {
logMessages.unregister();
}
}

@Test
public void testResponseErrorExceptionInEndpoint() {
LogMessageAccumulator logMessages = new LogMessageAccumulator();
try {
// Don't show the exception in the test execution log
logMessages.registerTo(RemoteEndpoint.class);

TestEndpoint endp = new TestEndpoint() {
@Override
public CompletableFuture<Object> request(String method, Object parameter) {
throw new ResponseErrorException(new ResponseError(ResponseErrorCode.InvalidParams, "Direct Throw", "data"));
}
};
TestMessageConsumer consumer = new TestMessageConsumer();
RemoteEndpoint endpoint = new RemoteEndpoint(consumer, endp);

endpoint.consume(init(new RequestMessage(), it -> {
it.setId("1");
it.setMethod("foo");
it.setParams("myparam");
}));

ResponseMessage response = (ResponseMessage) consumer.messages.get(0);
assertEquals("Direct Throw", response.getError().getMessage());
assertEquals(ResponseErrorCode.InvalidParams.getValue(), response.getError().getCode());
String data = (String) response.getError().getData();
assertEquals("data", data);
} finally {
logMessages.unregister();
}
}

@Test
public void testResponseErrorExceptionFromFutureInEndpoint() {
LogMessageAccumulator logMessages = new LogMessageAccumulator();
try {
// Don't show the exception in the test execution log
logMessages.registerTo(RemoteEndpoint.class);

TestEndpoint endp = new TestEndpoint() {
@Override
public CompletableFuture<Object> request(String method, Object parameter) {
CompletableFuture<Object> future = new CompletableFuture<>();
future.completeExceptionally(new ResponseErrorException(
new ResponseError(ResponseErrorCode.InvalidParams, "completeExceptionally", "data")));
return future;
}
};
TestMessageConsumer consumer = new TestMessageConsumer();
RemoteEndpoint endpoint = new RemoteEndpoint(consumer, endp);

endpoint.consume(init(new RequestMessage(), it -> {
it.setId("1");
it.setMethod("foo");
it.setParams("myparam");
}));

ResponseMessage response = (ResponseMessage) consumer.messages.get(0);
assertEquals("completeExceptionally", response.getError().getMessage());
assertEquals(ResponseErrorCode.InvalidParams.getValue(), response.getError().getCode());
String data = (String) response.getError().getData();
assertEquals("data", data);
} finally {
logMessages.unregister();
}
}

@Test
public void testExceptionInConsumer() throws Exception {
Expand Down

0 comments on commit 4c49a57

Please sign in to comment.