mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-11 03:13:51 +00:00
Merge pull request #1848 from AmirF194/fix/1814-tsserver-crash-silent-empty-result
Fix: surface a mid-indexing tsserver crash instead of returning {}
This commit is contained in:
3 files changed
+160
-2
No files matched your search
@@ -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`
|
||||
|
||||
@@ -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:
|
||||
"""
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user