-
-
Notifications
You must be signed in to change notification settings - Fork 90
lsp: Exit on unexpected errors #856
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
Conversation
janvhs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From reading the source, stopping the LSP on error cases, seems to resolve the case where the GC is overwhelmed. I think it is a way better solution compared to what I proposed in #855 ! :)
There is however an error in the type signature, which prevents me from fully trying it.
src/lsp/LSPClient.js
Outdated
| await Promise.all([ | ||
| this.stdin.close_async(null), | ||
| this.stdout.close_async(null), | ||
| this.stdin.close_async(null).catch(console.error), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk what argument is missing, but here is a link to the docs: https://gjs-docs.gjsify.org/classes/Gio_2_0.Gio.OutputStreamClass.html#close_async
(re.sonny.Workbench.Devel:5): re.sonny.Workbench.Devel-CRITICAL **: 14:15:28.821: TypeError: method Gio.OutputStream.close_async: At least 3 arguments required, but only 2 passed
_promisify/proto[asyncFunc]/<@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:450:35
_promisify/proto[asyncFunc]@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:448:16
stop@resource:///re/sonny/Workbench/lsp/LSPClient.js:88:18
_read@resource:///re/sonny/Workbench/lsp/LSPClient.js:220:19
async*_start_process@resource:///re/sonny/Workbench/lsp/LSPClient.js:164:10
start@resource:///re/sonny/Workbench/lsp/LSPClient.js:49:10
setup@resource:///re/sonny/Workbench/langs/javascript/javascript.js:13:8
JavaScriptDocument@resource:///re/sonny/Workbench/langs/javascript/JavaScriptDocument.js:10:17
Window@resource:///re/sonny/Workbench/window.js:81:31
openDemo@resource:///re/sonny/Workbench/Library/Library.js:105:26
Library/</<@resource:///re/sonny/Workbench/Library/Library.js:42:15
EntryRow/<@resource:///re/sonny/Workbench/Library/EntryRow.js:25:12
_init/GLib.MainLoop.prototype.runAsync/</<@resource:///org/gnome/gjs/modules/core/overrides/GLib.js:266:34
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks and fixed
| try { | ||
| return JSON.parse(str); | ||
| } catch (err) { | ||
| await this.stop(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stopping the LSP on an error is a good idea, but not re-throwing the error could potentially make debugging harder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Knowing that the input isn't valid JSON isn't particularly helpful, so one has to investigate anyway.
Working with LSP servers is rather cumbersome 😞
356e08c to
a9b16d2
Compare
Fixes #855