Skip to content

Commit d142f58

Browse files
sebthomsebthom
authored
fix: gracefully handle LSP errors in OpenDeclarationHyperlinkDetector (#1380)
Wrap definition/declaration/typeDefinition/implementation requests with exceptionally(...) so per-call failures resolve to harmless results instead of breaking the merge. Fixes #1169. Supersedes #1186
1 parent 78d3b10 commit d142f58

2 files changed

Lines changed: 116 additions & 7 deletions

File tree

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/definition/HyperlinkDetectorErrorHandlingTest.java‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation
11+
*******************************************************************************/
12+
package org.eclipse.lsp4e.test.definition;
13+
14+
import static org.junit.Assert.*;
15+
16+
import java.util.List;
17+
import java.util.concurrent.CompletableFuture;
18+
19+
import org.eclipse.core.resources.IFile;
20+
import org.eclipse.jface.text.ITextViewer;
21+
import org.eclipse.jface.text.Region;
22+
import org.eclipse.jface.text.hyperlink.IHyperlink;
23+
import org.eclipse.lsp4e.operations.declaration.OpenDeclarationHyperlinkDetector;
24+
import org.eclipse.lsp4e.test.utils.AbstractTestWithProject;
25+
import org.eclipse.lsp4e.test.utils.TestUtils;
26+
import org.eclipse.lsp4e.tests.mock.MockLanguageServer;
27+
import org.eclipse.lsp4e.tests.mock.MockTextDocumentService;
28+
import org.eclipse.lsp4j.DeclarationParams;
29+
import org.eclipse.lsp4j.ImplementationParams;
30+
import org.eclipse.lsp4j.Location;
31+
import org.eclipse.lsp4j.LocationLink;
32+
import org.eclipse.lsp4j.Position;
33+
import org.eclipse.lsp4j.Range;
34+
import org.eclipse.lsp4j.ServerCapabilities;
35+
import org.eclipse.lsp4j.TypeDefinitionParams;
36+
import org.eclipse.lsp4j.jsonrpc.messages.Either;
37+
import org.junit.Test;
38+
39+
public class HyperlinkDetectorErrorHandlingTest extends AbstractTestWithProject {
40+
41+
private final OpenDeclarationHyperlinkDetector detector = new OpenDeclarationHyperlinkDetector();
42+
43+
@Override
44+
protected ServerCapabilities getServerCapabilities() {
45+
// Ensure providers are enabled to exercise all branches
46+
var caps = MockLanguageServer.defaultServerCapabilities();
47+
caps.setDefinitionProvider(true);
48+
caps.setTypeDefinitionProvider(true);
49+
caps.setDeclarationProvider(true);
50+
caps.setImplementationProvider(true);
51+
return caps;
52+
}
53+
54+
@Test
55+
public void testDefinitionRemainsWhenTypeDefinitionErrors() throws Exception {
56+
MockLanguageServer.INSTANCE.setTextDocumentService(
57+
// Simulate server error for typeDefinition (mirrors issue
58+
// https://github.com/eclipse-lsp4e/lsp4e/issues/1169)
59+
new MockTextDocumentService(MockLanguageServer.INSTANCE::buildMaybeDelayedFuture) {
60+
@Override
61+
public CompletableFuture<Either<List<? extends Location>, List<? extends LocationLink>>> typeDefinition(
62+
TypeDefinitionParams params) {
63+
var f = new CompletableFuture<Either<List<? extends Location>, List<? extends LocationLink>>>();
64+
f.completeExceptionally(
65+
new RuntimeException("unexpected error during typeDefinition retrieval"));
66+
return f;
67+
}
68+
69+
@Override
70+
public CompletableFuture<Either<List<? extends Location>, List<? extends LocationLink>>> implementation(
71+
ImplementationParams params) {
72+
throw new RuntimeException("unexpected error during implementation retrieval");
73+
}
74+
75+
@Override
76+
public CompletableFuture<Either<List<? extends Location>, List<? extends LocationLink>>> declaration(
77+
DeclarationParams params) {
78+
throw new RuntimeException("unexpected error during declaration retrieval");
79+
}
80+
});
81+
82+
// ensure TextDocumentService is faulty
83+
assertThrows(RuntimeException.class,
84+
() -> MockLanguageServer.INSTANCE.getTextDocumentService().declaration(null));
85+
assertThrows(RuntimeException.class,
86+
() -> MockLanguageServer.INSTANCE.getTextDocumentService().implementation(null));
87+
assertTrue(
88+
MockLanguageServer.INSTANCE.getTextDocumentService().typeDefinition(null).isCompletedExceptionally());
89+
90+
// Configure 1 good definition result
91+
MockLanguageServer.INSTANCE.setDefinition(List.of( //
92+
new Location("file://def", new Range(new Position(0, 0), new Position(0, 10))), //
93+
new Location("file://def", new Range(new Position(1, 10), new Position(1, 20)))));
94+
95+
IFile file = TestUtils.createUniqueTestFile(project, "Example Text");
96+
ITextViewer viewer = TestUtils.openTextViewer(file);
97+
98+
IHyperlink[] links = detector.detectHyperlinks(viewer, new Region(0, 0), true);
99+
100+
// Expected: 1 link (from definition) even if typeDefinition fails
101+
assertNotNull("Hyperlinks should not be null when definition succeeds despite typeDefinition error", links);
102+
assertEquals(2, links.length);
103+
}
104+
}

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/declaration/OpenDeclarationHyperlinkDetector.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,24 +74,29 @@ private static record LabeledLocations(String label,
7474
final int offset = region.getOffset();
7575

7676
final CompletableFuture<List<LSBasedHyperlink>> request = CACHE.computeIfAbsent(document, offset, () -> {
77-
var definitions = LanguageServers.forDocument(document)
77+
final var definitions = LanguageServers.forDocument(document)
7878
.withCapability(ServerCapabilities::getDefinitionProvider)
7979
.collectAll(ls -> ls.getTextDocumentService().definition(LSPEclipseUtils.toDefinitionParams(params))
80-
.thenApply(l -> new LabeledLocations(Messages.definitionHyperlinkLabel, l)));
80+
.thenApply(l -> new LabeledLocations(Messages.definitionHyperlinkLabel, l))
81+
.exceptionally(err -> new LabeledLocations(Messages.definitionHyperlinkLabel, null)));
8182
final var declarations = LanguageServers.forDocument(document)
82-
.withCapability(ServerCapabilities::getDeclarationProvider).collectAll(
83-
ls -> ls.getTextDocumentService().declaration(LSPEclipseUtils.toDeclarationParams(params))
84-
.thenApply(l -> new LabeledLocations(Messages.declarationHyperlinkLabel, l)));
83+
.withCapability(ServerCapabilities::getDeclarationProvider)
84+
.collectAll(ls -> ls.getTextDocumentService()
85+
.declaration(LSPEclipseUtils.toDeclarationParams(params))
86+
.thenApply(l -> new LabeledLocations(Messages.declarationHyperlinkLabel, l))
87+
.exceptionally(err -> new LabeledLocations(Messages.declarationHyperlinkLabel, null)));
8588
final var typeDefinitions = LanguageServers.forDocument(document)
8689
.withCapability(ServerCapabilities::getTypeDefinitionProvider)
8790
.collectAll(ls -> ls.getTextDocumentService()
8891
.typeDefinition(LSPEclipseUtils.toTypeDefinitionParams(params))
89-
.thenApply(l -> new LabeledLocations(Messages.typeDefinitionHyperlinkLabel, l)));
92+
.thenApply(l -> new LabeledLocations(Messages.typeDefinitionHyperlinkLabel, l))
93+
.exceptionally(err -> new LabeledLocations(Messages.typeDefinitionHyperlinkLabel, null)));
9094
final var implementations = LanguageServers.forDocument(document)
9195
.withCapability(ServerCapabilities::getImplementationProvider)
9296
.collectAll(ls -> ls.getTextDocumentService()
9397
.implementation(LSPEclipseUtils.toImplementationParams(params))
94-
.thenApply(l -> new LabeledLocations(Messages.implementationHyperlinkLabel, l)));
98+
.thenApply(l -> new LabeledLocations(Messages.implementationHyperlinkLabel, l))
99+
.exceptionally(err -> new LabeledLocations(Messages.implementationHyperlinkLabel, null)));
95100

96101
CompletableFuture<List<LabeledLocations>> combined = LanguageServers.addAll(
97102
LanguageServers.addAll(LanguageServers.addAll(definitions, declarations), typeDefinitions),

0 commit comments

Comments
 (0)