Closed davidanthoff closed 4 years ago
Merging #114 into master will decrease coverage by
4.96%
. The diff coverage is82.35%
.
@@ Coverage Diff @@
## master #114 +/- ##
=========================================
- Coverage 80.16% 75.2% -4.97%
=========================================
Files 5 5
Lines 595 621 +26
=========================================
- Hits 477 467 -10
- Misses 118 154 +36
Impacted Files | Coverage Δ | |
---|---|---|
src/server.jl | 80% <ø> (-8.89%) |
:arrow_down: |
src/symbols.jl | 84.5% <100%> (-0.39%) |
:arrow_down: |
src/SymbolServer.jl | 84.14% <80%> (-1.31%) |
:arrow_down: |
src/utils.jl | 33.62% <0%> (-26.55%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 220becd...ffcfa96. Read the comment docs.
This fixes another problem from crash reporting. Sometimes the symbol server crashes with a segfault because a package that we load has some really buggy code. Our
try catch
for these situations doesn't help. In general, we don't want to send a crash report for those cases: it is not our fault, so we shouldn't count this as a failure on our end.This PR (and the companion PRs in LS and the client) works around this by adding a back communication channel from the symbol server to the LS process. The symbol server there notifies the LS "I will try to load package X now". If the symserver then crashes, the LS knows that it was a segfault in a package and sends a special trace event via telemetry. As soon as the symserver is done loading the user package it sends a message "I'm done loading package X" to the LS, so the LS now knows that if the symserver crashes after that notification, there is just a bug in the symserver that we should report via crash reporting.
As a small side bonus we get nicer progress reporting for the indexing process :)
Requires https://github.com/julia-vscode/LanguageServer.jl/pull/591 and https://github.com/julia-vscode/julia-vscode/pull/1043.
This is finicky to test, but I did try out a whole variety of different crash situations and it seems to work.