ruff
d8a0f0ae - [ty] Fix Salsa panic propagation (#24141)

Commit
41 days ago
[ty] Fix Salsa panic propagation (#24141) ## Summary This PR fixes probably the most likely case why users saw https://github.com/astral-sh/ty/issues/1565 in their IDE. I added handling to convert panics to diagnostics in https://github.com/astral-sh/ruff/pull/17631 to `check_file_impl`. However, this was before `check_file_impl` became a Salsa query. We have to be a bit more careful, now that `check_file_impl` is a Salsa query. 1. We have to add an untracked read whenever we suppress a panic because Salsa only carries over dependencies of queries that run to completion and not of queries that panic (the assumption is that all queries unwind). Adding an `untracked_read` ensures that the `check_file_impl` reruns after every change. This is more often than necessary, but it is the best we can do here without knowing the exact dependencies that were collected up to when `check_file_impl`'s dependency panicked. 2. Suppressing all Salsa panics in `check_files_impl` is unlikely to be what we want because it means the function still runs to completion even when the query was cancelled. Instead, we want to propagate a cancellation so that its `db` handle gets released as quickly as possible to unblock any pending mutation. However, we do need some special handling for Salsa's propagating panic to avoid regressing https://github.com/astral-sh/ruff/pull/17631. For a query `A` running on thread `a` that depends on query `B` running on thread `b`. If `B` panics, Salsa throws the original panic on thread `b` but throws a `Cancelled::PropagatingPanic` panic on thread `a`. Thread `b`'s panic is the more useful one because it contains the actual panic information. We already convert `b`'s panic to a `Diagnostic`, but we silently ignore any `Cancelled::PropagatingPanic`. This PR also creates a propagating panic to a diagnostic. While these panics don't contain any useful information, they at least indicate to a user that `A` was only partially checked. They should have a second diagnostic for `B` that contains the full panic information (unless `B` is a query that didn't run as part of `project.check`, e.g. `hover`). ## Test plan I used @AlexWaygood's panda-stubs reproducer and I was no longer able to reproduce the Salsa panic. I plan on doing some CLI `--watch` testing tomorrow morning but this should block code review.
Author
Parents
Loading