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

Unhandled null in getWorkDoneProgress() breaks Vim usage #2936

Closed
Tachi107 opened this issue Oct 30, 2023 · 3 comments · Fixed by #2938
Closed

Unhandled null in getWorkDoneProgress() breaks Vim usage #2936

Tachi107 opened this issue Oct 30, 2023 · 3 comments · Fixed by #2938
Labels

Comments

@Tachi107
Copy link

Hi all, pull request #2776 (commit 2774467) broke jdtls for me when using it in Vim with the vim9 lsp plugin. More specifically, the getWorkDoneProgress() method is returning null, which causes a null pointer exception when using it in the new isWorkDoneProgressSupported() method.

Here's the error message shown by the lsp plugin when using jdtls 1.27.0:

'jdtls' Language Server Messages
================================
10/30/23 11:34:42: [Error]: 30 ott 2023, 11:34:42 Unhandled error
Cannot invoke "java.lang.Boolean.booleanValue()" because the return value of "org.eclipse.lsp4j.WindowClientCapabilities.getWorkDoneProgress()" is null
java.lang.NullPointerException: Cannot invoke "java.lang.Boolean.booleanValue()" because the return value of "org.eclipse.lsp4j.WindowClientCapabilities.getWorkDoneProgress()" is null
	at org.eclipse.jdt.ls.core.internal.preferences.ClientPreferences.isWorkDoneProgressSupported(ClientPreferences.java:185)
	at org.eclipse.jdt.ls.core.internal.handlers.ProgressReporterManager$ProgressReporter.sendStatus(ProgressReporterManager.java:262)
	at org.eclipse.jdt.ls.core.internal.handlers.ProgressReporterManager$ProgressReporter.sendProgress(ProgressReporterManager.java:239)
	at org.eclipse.jdt.ls.core.internal.handlers.ProgressReporterManager$ProgressReporter.done(ProgressReporterManager.java:222)
	at org.eclipse.jdt.ls.core.internal.handlers.ProgressReporterManager$MulticastProgressReporter.done(ProgressReporterManager.java:102)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:74)

The issue is still present in the 1.29.0 release (and I suspect in the master branch too).

What's the root of the issue here? Is the vim9 lsp plugin doing something wrong, or should jdtls account for nulls returned by getWorkDoneProgress()?

Cc: @hopehadfield

@rgrunber
Copy link
Contributor

rgrunber commented Oct 30, 2023

Yes, JDT-LS can certainly fix the issue.

The LSP spec defines https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#clientCapabilities :

window?: {
		/**
		 * It indicates whether the client supports server initiated
		 * progress using the `window/workDoneProgress/create` request.
		 *
		 * The capability also controls Whether client supports handling
		 * of progress notifications. If set servers are allowed to report a
		 * `workDoneProgress` property in the request specific server
		 * capabilities.
		 *
		 * @since 3.15.0
		 */
		workDoneProgress?: boolean;

So the value can be true, false, or null (optional).

JDT-LS should definitely handle the null the same way we would handle false.

The client could also work around the issue by setting workdoneProgress to false at https:/yegappan/lsp/blob/38970403dc4d602056ea4028664206ffce74e82a/autoload/lsp/capabilities.vim . If the client supports this capability, they'll need to set it to true (not supported currently in the client).

@hopehadfield feel free to fix on our end.

@Tachi107
Copy link
Author

Tachi107 commented Oct 30, 2023 via email

@rgrunber
Copy link
Contributor

@Tachi107 , Yes exactly. I think if you're receiving the comment updates by e-mail, you don't see the subsequent edits I do. I updated once i discovered the lsp client mentioned ignores $/progress.

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

Successfully merging a pull request may close this issue.

2 participants