Unwrap InvocationTargetException which are ResponseErrorException

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 Fixed issues: <https://github.com/eclipse-lsp4j/lsp4j/milestone/34?closed=1>
 
+  * The exception handling around throwing `ResponseErrorException` has been improved to ensure that it is unwrapped to the expected `ResponseError` on the receiving side.
+  In addition, `@JsonDelegate`s that throw exceptions have their checked exceptions wrapped in the more narrow `IllegalStateException` instead of a `RuntimeException`.
+    * See [#802](https://github.com/eclipse-lsp4j/lsp4j/issues/802) for detailed discussion.
+
 Breaking API changes:
 
 Nightly japicmp report: <https://download.eclipse.org/lsp4j/builds/main/japicmp-report/>

documentation/jsonrpc.md

@@ -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();
    }

org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/services/GenericEndpoint.java

@@ -1,5 +1,5 @@
 /******************************************************************************
- * Copyright (c) 2016 TypeFox and others.
+ * Copyright (c) 2016, 2024 TypeFox and others.
  * 
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -63,8 +64,10 @@ 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) {
-					throw new RuntimeException(e);
+				} catch (InvocationTargetException e) {
+					throw new CompletionException(e.getCause());
+				} catch (IllegalAccessException e) {
+					throw new CompletionException(e);
 				}
 			};
 			if (methodHandlers.put(methodInfo.name, handler) != null) {
@@ -80,7 +83,7 @@ protected void recursiveFindRpcMethods(Object current, Set<Class<?>> visited, Se
 					LOG.fine("A delegate object is null, jsonrpc methods of '" + method + "' are ignored");
 				}
 			} catch (InvocationTargetException | IllegalAccessException e) {
-				throw new RuntimeException(e);
+				throw new IllegalStateException("An exception occurred while executing JsonDelegate method " + method, e);
 			}
 		});
 	}

org.eclipse.lsp4j.jsonrpc/src/test/java/org/eclipse/lsp4j/jsonrpc/test/IntegrationTest.java

@@ -1,5 +1,5 @@
 /******************************************************************************
- * Copyright (c) 2016 TypeFox and others.
+ * Copyright (c) 2016, 2024 TypeFox and others.
  * 
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -11,6 +11,9 @@
  ******************************************************************************/
 package org.eclipse.lsp4j.jsonrpc.test;
 
+import static org.eclipse.lsp4j.jsonrpc.json.MessageConstants.CONTENT_LENGTH_HEADER;
+import static org.eclipse.lsp4j.jsonrpc.json.MessageConstants.CRLF;
+
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.PipedInputStream;
@@ -34,16 +37,15 @@
 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;
 import org.eclipse.lsp4j.jsonrpc.validation.NonNull;
 import org.junit.Assert;
 import org.junit.Test;
 
-import static org.eclipse.lsp4j.jsonrpc.json.MessageConstants.CONTENT_LENGTH_HEADER;
-import static org.eclipse.lsp4j.jsonrpc.json.MessageConstants.CRLF;
-
 public class IntegrationTest {
 
 	private static final long TIMEOUT = 2000;
@@ -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;
 				});
@@ -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);
 	}

org.eclipse.lsp4j.jsonrpc/src/test/java/org/eclipse/lsp4j/jsonrpc/test/RemoteEndpointTest.java

@@ -1,5 +1,5 @@
 /******************************************************************************
- * Copyright (c) 2016 TypeFox and others.
+ * Copyright (c) 2016, 2024 TypeFox and others.
  * 
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -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;
@@ -219,6 +220,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 {