diff --git a/CHANGELOG.md b/CHANGELOG.md index bba1abc6..30a50473 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,11 @@ Status of the `main` branch. Changes prior to the next official version change w indicate it was incomplete. Serena now declares work-done progress support and waits for the work Metals reports, bounded by the new `indexing_timeout`, `indexing_start_grace` and `indexing_quiet_period` settings + - Fix: a `tsserver` crash mid-indexing (e.g. a V8 heap OOM) sent the same `$/progress` "end" + event as a normal completion, so `find_referencing_symbols` and other cross-file queries + silently returned an empty result instead of surfacing the crash. The crash is now detected + independently via the `window/logMessage` notification tsserver already sends, and the + affected wait now raises instead of reporting success (#1814) * Dependencies: - Remove the redundant `dotenv` dependency; the `dotenv` module is provided by `python-dotenv` diff --git a/src/solidlsp/language_servers/typescript_language_server.py b/src/solidlsp/language_servers/typescript_language_server.py index 063b2524..c02cf13c 100644 --- a/src/solidlsp/language_servers/typescript_language_server.py +++ b/src/solidlsp/language_servers/typescript_language_server.py @@ -4,6 +4,7 @@ Provides TypeScript specific instantiation of the LanguageServer class. Contains import logging import os +import re import shutil import threading import time @@ -15,7 +16,9 @@ from sensai.util.logging import LogTime from solidlsp import ls_types from solidlsp.ls import LanguageServerDependencyProvider, LanguageServerDependencyProviderSinglePath, SolidLanguageServer from solidlsp.ls_config import LanguageServerConfig +from solidlsp.ls_exceptions import SolidLSPException from solidlsp.ls_utils import PlatformId, PlatformUtils +from solidlsp.lsp_protocol_handler.lsp_types import MessageType from solidlsp.settings import SolidLSPSettings from .common import RuntimeDependency, RuntimeDependencyCollection, build_npm_install_command @@ -65,6 +68,21 @@ def prefer_non_node_modules_definition(definitions: list[ls_types.Location]) -> return definitions[0] +class TypeScriptServerCrashedError(SolidLSPException): + """Raised when tsserver reported its own abnormal exit via window/logMessage. + + typescript-language-server sends a $/progress "end" event for the in-flight + token as part of tearing its connection down after tsserver dies, which is + otherwise indistinguishable from a normal indexing completion. + """ + + +# Matches typescript-language-server's window/logMessage notification for an abnormal +# tsserver exit, e.g. "[lspserver] [tsclient] [tsserver] Exited. Code: null. Signal: SIGABRT". +# A clean shutdown does not produce this message. +_TSSERVER_EXITED_PATTERN = re.compile(r"\[tsserver\]\s+Exited\b", re.IGNORECASE) + + class TypeScriptLanguageServer(SolidLanguageServer): """ Provides TypeScript specific instantiation of the LanguageServer class. Contains various configurations and settings specific to TypeScript. @@ -114,14 +132,36 @@ class TypeScriptLanguageServer(SolidLanguageServer): self._active_progress_tokens: set[str] = set() self._indexing_complete = threading.Event() self._indexing_complete.set() # Initially set (no active work) + # set from window/logMessage when tsserver reports its own abnormal exit; + # a crash mid-indexing still drains _active_progress_tokens via a $/progress + # "end" event, so that alone cannot distinguish a crash from real completion + self._crash_message: str | None = None + + def _raise_if_crashed(self) -> None: + if self._crash_message is not None: + raise TypeScriptServerCrashedError(self._crash_message) + + @staticmethod + def _tsserver_exit_message(msg: dict) -> str | None: + """:return: the log text if ``msg`` is tsserver reporting its own abnormal exit, else None.""" + if msg.get("type") != MessageType.Error: + return None + message_text = str(msg.get("message", "")) + if _TSSERVER_EXITED_PATTERN.search(message_text): + return message_text + return None def wait_for_indexing(self, timeout: float) -> bool: """Block until all $/progress tokens complete. :param timeout: Maximum seconds to wait. :return: True if indexing completed, False on timeout. + :raises TypeScriptServerCrashedError: if tsserver reported an abnormal exit. """ - return self._indexing_complete.wait(timeout=timeout) + result = self._indexing_complete.wait(timeout=timeout) + if result: + self._raise_if_crashed() + return result def _wait_for_indexing_start_or_completion(self, timeout: float, start_grace: float | None = None) -> bool: """Wait until TypeScript indexing has started and drained, or provably never started. @@ -129,6 +169,7 @@ class TypeScriptLanguageServer(SolidLanguageServer): :param timeout: Maximum seconds to wait once active indexing progress is observed. :param start_grace: Maximum seconds to wait for progress to begin after opening files. :return: True if indexing completed or no progress began within the grace period, False on timeout. + :raises TypeScriptServerCrashedError: if tsserver reported an abnormal exit. """ grace = self.INDEXING_START_GRACE if start_grace is None else start_grace @@ -139,6 +180,7 @@ class TypeScriptLanguageServer(SolidLanguageServer): if self._active_progress_tokens: break if self._indexing_complete.is_set(): + self._raise_if_crashed() return True time.sleep(0.05) @@ -147,6 +189,7 @@ class TypeScriptLanguageServer(SolidLanguageServer): has_active_progress = bool(self._active_progress_tokens) if not has_active_progress: self._indexing_complete.set() + self._raise_if_crashed() return True # wait for active progress to drain @@ -382,6 +425,10 @@ class TypeScriptLanguageServer(SolidLanguageServer): def window_log_message(msg: dict) -> None: log.info(f"LSP: window/logMessage: {msg}") + crash_text = self._tsserver_exit_message(msg) + if crash_text is not None: + log.warning(f"tsserver reported an abnormal exit: {crash_text}") + self._crash_message = f"tsserver exited abnormally: {crash_text}" def handle_typescript_version(params: dict) -> None: """ diff --git a/test/solidlsp/test_typescript_timeout_policy.py b/test/solidlsp/test_typescript_timeout_policy.py index ad2274b0..f71867ca 100644 --- a/test/solidlsp/test_typescript_timeout_policy.py +++ b/test/solidlsp/test_typescript_timeout_policy.py @@ -17,7 +17,7 @@ from solidlsp.language_servers.svelte_language_server import ( SvelteLanguageServer, SvelteTypeScriptServer, ) -from solidlsp.language_servers.typescript_language_server import TypeScriptLanguageServer +from solidlsp.language_servers.typescript_language_server import TypeScriptLanguageServer, TypeScriptServerCrashedError from solidlsp.settings import SolidLSPSettings @@ -31,6 +31,7 @@ def _bare_ts_server(cls: type[TypeScriptLanguageServer], custom_settings: dict | server._active_progress_tokens = set() server._indexing_complete = threading.Event() server._indexing_complete.set() # mirrors __init__: initially no active work + server._crash_message = None # mirrors __init__: no crash observed yet server._custom_settings = SolidLSPSettings.CustomLSSettings(custom_settings) return server @@ -149,6 +150,111 @@ class TestWaitForIndexingStartOrCompletion: timer.cancel() +class TestTsserverCrashDetection: + """oraios/serena#1814: typescript-language-server sends a $/progress "end" for the + in-flight token as part of tearing its connection down after tsserver dies, which + _wait_for_indexing_start_or_completion previously could not distinguish from a real + completion: find_referencing_symbols returned {} with isError: false instead of + surfacing the crash. window/logMessage does carry the crash independently; these + tests drive the exact begin/crash-log/end sequence from the reporter's log and pin + that every wait path now raises instead of reporting silent success. + """ + + def test_tsserver_exit_message_matches_sigabrt_line(self) -> None: + msg = {"type": 1, "message": "[lspserver] [tsclient] [tsserver] Exited. Code: null. Signal: SIGABRT"} + assert TypeScriptLanguageServer._tsserver_exit_message(msg) == msg["message"] + + def test_tsserver_exit_message_ignores_non_error_type(self) -> None: + # type 3 (Info) is what a clean shutdown/log line would carry, not type 1 (Error) + msg = {"type": 3, "message": "[lspserver] [tsclient] [tsserver] Exited. Code: null. Signal: SIGABRT"} + assert TypeScriptLanguageServer._tsserver_exit_message(msg) is None + + def test_tsserver_exit_message_ignores_unrelated_error(self) -> None: + msg = {"type": 1, "message": "Some other tsserver error unrelated to process exit"} + assert TypeScriptLanguageServer._tsserver_exit_message(msg) is None + + def test_raise_if_crashed_is_a_noop_when_no_crash_observed(self) -> None: + server = _bare_ts_server(TypeScriptLanguageServer) + server._raise_if_crashed() # must not raise + + def test_raise_if_crashed_raises_with_recorded_message(self) -> None: + server = _bare_ts_server(TypeScriptLanguageServer) + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._raise_if_crashed() + + def test_wait_for_indexing_raises_when_progress_end_follows_a_crash(self) -> None: + """Drives the reporter's exact sequence: $/progress begin, the SIGABRT window/logMessage, + then $/progress end (teardown), the same "end" that previously read as success. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._active_progress_tokens.add("646a4e19") + + # window/logMessage handler's effect: record the crash independently of progress state + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + # $/progress end (teardown): drains the token and sets indexing_complete, exactly as a + # real completion would + server._active_progress_tokens.discard("646a4e19") + server._indexing_complete.set() + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server.wait_for_indexing(timeout=1.0) + + def test_wait_for_indexing_returns_true_on_clean_completion(self) -> None: + """Same $/progress begin/end shape as the crash test, without a crash message: must still + return True, not regress the ordinary success path. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._active_progress_tokens.add("646a4e19") + server._active_progress_tokens.discard("646a4e19") + server._indexing_complete.set() + + assert server.wait_for_indexing(timeout=1.0) is True + + def test_wait_for_indexing_start_or_completion_raises_via_early_is_set_branch(self) -> None: + """Covers the early "if self._indexing_complete.is_set(): return True" branch, which is + reached (not the tail wait_for_indexing() call) when begin+crash+end all land before the + first poll iteration observes an active token. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + server._indexing_complete.set() # progress already drained by the time we start waiting + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._wait_for_indexing_start_or_completion(timeout=1.0, start_grace=1.0) + + def test_wait_for_indexing_start_or_completion_raises_via_absent_progress_branch(self) -> None: + """Covers the "treat absent progress as ready" branch: no active token observed within the + start grace, but a crash was already recorded. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._wait_for_indexing_start_or_completion(timeout=1.0, start_grace=0.05) + + def test_wait_for_cross_file_references_propagates_the_crash(self) -> None: + """The actual call path find_referencing_symbols drives (ls.py's SymbolLocationRequest.execute + calls this with no surrounding try/except) must raise, not silently log "indexing complete" + and let the caller receive an empty result. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server._has_waited_for_cross_file_references = False + server.expect_indexing() + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._wait_for_cross_file_references_if_needed() + + # the crash must be reported, not swallowed into a false "we've already waited" state + assert server._has_waited_for_cross_file_references is False + + class TestWaitForCrossFileReferencesUsesConfiguredGrace: """The actual find-references call path (not just the helper in isolation) must honor indexing_start_grace: this is the mechanism behind oraios/serena#1586, where a large