Skip to content

Commit

Permalink
Use more specific exception when @JsonDelegates throw an exception
Browse files Browse the repository at this point in the history
This exception is thrown when the endpoint is created, not as a result of
a receiving a message. This is generally a coding error, so make this
exception the same as the other exceptions thrown by recursiveFindRpcMethods.

Included is adding additional tests to ensure coverage of some
previously missing uses cases.

Code cleanup done as part of #802
  • Loading branch information
jonahgraham committed Feb 20, 2024
1 parent aa092c0 commit aa0bb9e
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,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);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -42,6 +42,26 @@ public OtherThing doDelegate() {
}

}

public static class DelegateThrows {
@JsonDelegate
public OtherThing doDelegate() throws Exception {
throw new Exception("Exception in doDelegate");
}
}

public static class DuplicateNames1 {
@JsonNotification
public void duplicateName() {
Assert.fail("Should be unreachable");
}
}
public static class DuplicateNames2 {
@JsonNotification
public void duplicateName() {
Assert.fail("Should be unreachable");
}
}

public static class Bar {

Expand Down Expand Up @@ -79,7 +99,25 @@ public void testSimple() {

Assert.assertEquals(2, foo.calls);
}


@Test
public void testDelegateThrows() {
DelegateThrows delegateThrows = new DelegateThrows();
try {
new GenericEndpoint(delegateThrows);
Assert.fail("Find delegate methods did not have expected exception raised when FooThrows.doDelegate was called.");
} catch (IllegalStateException e) {
Throwable t = e;
do {
if ("Exception in doDelegate".equals(t.getMessage())) {
// test passes
break;
}
} while ((t = t.getCause()) != null);
Assert.assertNotNull("Failed to find excepted exception in cause chain", t);
}
}

@Test
public void testMultiServices() {
Foo foo = new Foo();
Expand All @@ -93,6 +131,18 @@ public void testMultiServices() {
Assert.assertEquals(1, bar.calls);
}

@Test
public void testDuplicateNamesGeneratesError() {
DuplicateNames1 duplicateNames1 = new DuplicateNames1();
DuplicateNames2 duplicateNames2 = new DuplicateNames2();
try {
new GenericEndpoint(Arrays.asList(duplicateNames1, duplicateNames2));
Assert.fail("Expected exception about duplicated names not thrown");
} catch (IllegalStateException e) {
// test passes
}
}

@Test
public void testUnexpectedParams() {
LogMessageAccumulator logMessages = new LogMessageAccumulator();
Expand Down

0 comments on commit aa0bb9e

Please sign in to comment.