fix(lsp): handle get diagnostic errors better #14776
Conversation
cli/tsc/99_main_compiler.js
Outdated
| ...languageService.getSuggestionDiagnostics(specifier), | ||
| ...languageService.getSyntacticDiagnostics(specifier), | ||
| ]; | ||
| } catch { |
There was a problem hiding this comment.
Huh, interesting. Would it make sense to do this per method call? I guess not since these are transient errors? What kind of errors get thrown?
There was a problem hiding this comment.
I tried it, but if one failed, they all failed... it throws a {} (yeah, just an object and I can't find where it is actually occurring in typescript.js, but it just fails some point, I suspect because the internal state of stuff is temporarily wrong)
There was a problem hiding this comment.
It would be nice to know exactly what’s happening, but this seems fine to do.
There was a problem hiding this comment.
Looking at all the code, I suspect it is some sort of debug logging/failure that is occurring, related to an attempt to find a source file, but it being in a bad state it non-critically debug errors, but because we don't fully implement the debug host, instead of it being something meaningful handled by the TypeScript internal debugger, it bubbles up as a {}.
One thing we don't do is validate on the getDiagnostics that all the specifiers have source files... actually that is an interesting point, I will look into that. It could be we just need to go grab any missing source files, where we are asking for diagnostics for source files that aren't in tsc yet.
There was a problem hiding this comment.
ok, found it, it was ts.OperationCanceledException being thrown, which we should just swallow like we do with the OperationCanceledError, which explains the transient nature of the error. I have refactored it again to better handle that error.
There was a problem hiding this comment.
Perfect! Thanks for looking into it.
This handles errors in
getDiagnosticsrequest where there are transient errors in getting the diagnostics which cause an output of:In the debug console, which is distracting to users and doesn't impact the usage of the extension. Also, when one specifier causes an issue, all requested specifiers get dropped, which isn't ideal. This will handle the whole failure of getting diagnostics better.