Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XMLHover.getTextHover() shouldn't log CancellationExceptions #1598

Open
fbricon opened this issue Nov 10, 2023 · 1 comment
Open

XMLHover.getTextHover() shouldn't log CancellationExceptions #1598

fbricon opened this issue Nov 10, 2023 · 1 comment

Comments

@fbricon
Copy link
Contributor

fbricon commented Nov 10, 2023

I tried using the lemminx-maven jars with vscode-xml, and saw this exception in the output view:

[Error - 09:50:29] Nov 10, 2023 09:50:29 org.eclipse.lemminx.services.XMLHover getTextHover()
Message: While performing IHoverParticipant#onText
java.util.concurrent.CancellationException
	at org.eclipse.lsp4j.jsonrpc.CompletableFutures$FutureCancelChecker.checkCanceled(CompletableFutures.java:59)
	at org.eclipse.lemminx.extensions.maven.participants.hover.MavenHoverParticipant.collectArtifactDescription(MavenHoverParticipant.java:381)
	at org.eclipse.lemminx.extensions.maven.participants.hover.MavenHoverParticipant.onText(MavenHoverParticipant.java:153)
	at org.eclipse.lemminx.services.XMLHover.getTextHover(XMLHover.java:214)
	at org.eclipse.lemminx.services.XMLHover.doHover(XMLHover.java:99)
	at org.eclipse.lemminx.services.XMLLanguageService.doHover(XMLLanguageService.java:185)
	at org.eclipse.lemminx.XMLTextDocumentService.lambda$hover$5(XMLTextDocumentService.java:281)
	at org.eclipse.lemminx.commons.ModelTextDocuments.lambda$computeModelAsync$0(ModelTextDocuments.java:118)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
	at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:483)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)

https://github.com/eclipse/lemminx/blob/d1d67ec09ad8ded638fc7c8ce1f52b72b33b8ec9/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLHover.java#L219 should probably re-throw CancellationException and let the client handle the cancelled request

@angelozerr
Copy link
Contributor

We did that for other LSP opertaion like completion https://github.com/eclipse/lemminx/blob/d1d67ec09ad8ded638fc7c8ce1f52b72b33b8ec9/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLCompletions.java#L334

The main problem with this strategy is that if one participant throws a cancelled exception it breaks other particpant result. Ex : if lemminx provides hover for XSD documentation and lemminx-maven throws a cancelled exception because some maven component are not loaded, the XSD documentation will not displayed.

Cancelled exception can occur in 2 usecase:

  • the DOMDocument has changed, so we should stop all participant process
  • one participant throws a cancelled exception (like lemminx maven) and we should not stop the hover process.

We need to take care of those 2 usecases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants