From a15833b22d3790927894a3717f04fa8d31ce7c44 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 27 Jul 2025 18:24:03 +0200 Subject: [PATCH] Improve error handling in language server communication * Introduce appropriate exceptions that allow errors to be sufficiently identified * If an error occurs in the read thread of the LS output stream, cancel all pending requests, setting an appropriate error * Propagate error information all the way to the original caller (in the tool code): If errors are due to LS termination, restart the server and retry --- src/serena/tools/tools_base.py | 22 ++++--- src/solidlsp/ls_exceptions.py | 37 +++++++++-- src/solidlsp/ls_handler.py | 111 +++++++++++++++++++++++---------- src/solidlsp/ls_request.py | 8 ++- 4 files changed, 128 insertions(+), 50 deletions(-) diff --git a/src/serena/tools/tools_base.py b/src/serena/tools/tools_base.py index 173726fc..c8163390 100644 --- a/src/serena/tools/tools_base.py +++ b/src/serena/tools/tools_base.py @@ -1,6 +1,5 @@ import inspect import os -import traceback from abc import ABC from collections.abc import Iterable from dataclasses import dataclass @@ -16,6 +15,7 @@ from serena.prompt_factory import PromptFactory from serena.symbol import LanguageServerSymbolRetriever from serena.util.class_decorators import singleton from serena.util.inspection import iter_subclasses +from solidlsp.ls_exceptions import LanguageServerException if TYPE_CHECKING: from serena.agent import LinesRead, MemoriesManager, SerenaAgent @@ -237,18 +237,24 @@ class Tool(Component): self.agent.reset_language_server() # apply the actual tool - result = apply_fn(**kwargs) + try: + result = apply_fn(**kwargs) + except LanguageServerException as e: + if e.is_language_server_terminated(): + log.error(f"Language server terminated while executing tool ({e}). Restarting the language server and retrying ...") + self.agent.reset_language_server() + result = apply_fn(**kwargs) + else: + raise + + # record tool usage self.agent.record_tool_usage_if_enabled(kwargs, result, self) except Exception as e: if not catch_exceptions: raise - msg = f"Error executing tool: {e}\n{traceback.format_exc()}" - log.error( - f"Error executing tool: {e}. " - f"Consider restarting the language server to solve this (especially, if it's a timeout of a symbolic operation)", - exc_info=e, - ) + msg = f"Error executing tool: {e}" + log.error(f"Error executing tool: {e}", exc_info=e) result = msg if log_call: diff --git a/src/solidlsp/ls_exceptions.py b/src/solidlsp/ls_exceptions.py index 0484da6b..a3bb0f11 100644 --- a/src/solidlsp/ls_exceptions.py +++ b/src/solidlsp/ls_exceptions.py @@ -1,15 +1,40 @@ """ -This module contains the exceptions raised by the Multilspy framework. +This module contains the exceptions raised by the framework. """ class LanguageServerException(Exception): - """ - Exceptions raised by the Multilspy framework. - """ - - def __init__(self, message: str): + def __init__(self, message: str, cause: Exception | None = None): """ Initializes the exception with the given message. + + :param message: the message describing the exception + :param cause: the original exception that caused this exception, if any. + For exceptions raised during request handling, this is typically + * an LSPError for errors returned by the LSP server + * LanguageServerTerminatedException for errors due to the language server having terminated. """ + self.cause = cause super().__init__(message) + + def is_language_server_terminated(self): + """ + :return: True if the exception is caused by the language server having terminated as indicated + by the causing exception being an instance of LanguageServerTerminatedException. + """ + from .ls_handler import LanguageServerTerminatedException + + return isinstance(self.cause, LanguageServerTerminatedException) + + def __str__(self) -> str: + """ + Returns a string representation of the exception. + """ + s = super().__str__() + if self.cause: + if "\n" in s: + s += "\n" + else: + s += " " + s += f"(caused by {self.cause})" + return s diff --git a/src/solidlsp/ls_handler.py b/src/solidlsp/ls_handler.py index 5e6754be..ab8a6772 100644 --- a/src/solidlsp/ls_handler.py +++ b/src/solidlsp/ls_handler.py @@ -12,6 +12,7 @@ from queue import Queue from typing import Any import psutil +from sensai.util.string import ToStringMixin from solidlsp.ls_exceptions import LanguageServerException from solidlsp.ls_request import LanguageServerRequest @@ -35,23 +36,50 @@ from solidlsp.lsp_protocol_handler.server import ( log = logging.getLogger(__name__) -class Request: +class LanguageServerTerminatedException(Exception): + """ + Exception raised when the language server process has terminated unexpectedly. + """ + + def __init__(self, message: str, cause: Exception | None = None) -> None: + super().__init__(message) + self.message = message + self.cause = cause + + def __str__(self) -> str: + return f"LanguageServerTerminatedException: {self.message}" + (f"; Cause: {self.cause}" if self.cause else "") + + +class Request(ToStringMixin): @dataclass class Result: payload: PayloadLike | None = None - error: Error | None = None + error: Exception | None = None def is_error(self) -> bool: return self.error is not None - def __init__(self) -> None: + def __init__(self, request_id: int, method: str) -> None: + self._request_id = request_id + self._method = method + self._status = "pending" self._result_queue = Queue() + def _tostring_includes(self) -> list[str]: + return ["_request_id", "_status", "_method"] + def on_result(self, params: PayloadLike) -> None: + self._status = "completed" self._result_queue.put(Request.Result(payload=params)) - def on_error(self, err: Error) -> None: + def on_error(self, err: Exception) -> None: + """ + :param err: the error that occurred while processing the request (typically an LSPError + for errors returned by the LS or LanguageServerTerminatedException if the error + is due to the language server process terminating unexpectedly). + """ + self._status = "error" self._result_queue.put(Request.Result(error=err)) def get_result(self, timeout: float | None = None) -> Result: @@ -75,10 +103,8 @@ class SolidLanguageServerHandler: notify: A LspNotification object that can be used to send notifications to the server. cmd: A string that represents the command to launch the language server process. process: A subprocess.Popen object that represents the language server process. - _received_shutdown: A boolean flag that indicates whether the client has received - a shutdown request from the server. request_id: An integer that represents the next available request id for the client. - _response_handlers: A dictionary that maps request ids to Request objects that + _pending_requests: A dictionary that maps request ids to Request objects that store the results or errors of the requests. on_request_handlers: A dictionary that maps method names to callback functions that handle requests from the server. @@ -109,10 +135,10 @@ class SolidLanguageServerHandler: self.process_launch_info = process_launch_info self.process = None - self._received_shutdown = False + self._is_shutting_down = False self.request_id = 1 - self._response_handlers: dict[Any, Request] = {} + self._pending_requests: dict[Any, Request] = {} self.on_request_handlers = {} self.on_notification_handlers = {} self.logger = logger @@ -176,12 +202,12 @@ class SolidLanguageServerHandler: # start threads to read stdout and stderr of the process threading.Thread( - target=self.run_forever, + target=self._read_ls_process_stdout, name="LSP-stdout-reader", daemon=True, ).start() threading.Thread( - target=self.run_forever_stderr, + target=self._read_ls_process_stderr, name="LSP-stderr-reader", daemon=True, ).start() @@ -261,10 +287,10 @@ class SolidLanguageServerHandler: """ Perform the shutdown sequence for the client, including sending the shutdown request to the server and notifying it of exit """ + self._is_shutting_down = True self._log("Sending shutdown request to server") self.send.shutdown() self._log("Received shutdown response from server") - self._received_shutdown = True self._log("Sending exit notification to server") self.notify.exit() self._log("Sent exit notification to server") @@ -279,27 +305,26 @@ class SolidLanguageServerHandler: @staticmethod def _read_bytes_from_process(process, stream, num_bytes): """Read exactly num_bytes from process stdout""" - if process.poll() is not None: - # Process has terminated, check if we can still read - pass - data = b"" while len(data) < num_bytes: chunk = stream.read(num_bytes - len(data)) if not chunk: if process.poll() is not None: - raise EOFError(f"Process terminated. Expected {num_bytes} bytes, got {len(data)}") - # Process still running but no data available yet - time.sleep(0.01) # Small delay + raise LanguageServerTerminatedException( + f"Process terminated while trying to read response (read {num_bytes} of {len(data)} bytes before termination)" + ) + # Process still running but no data available yet, retry after a short delay + time.sleep(0.01) continue data += chunk return data - def run_forever(self) -> bool: + def _read_ls_process_stdout(self) -> None: """ Continuously read from the language server process stdout and handle the messages invoking the registered response and notification handlers """ + exception: Exception | None = None try: while self.process and self.process.stdout and self.process.stdout.readable(): line = self.process.stdout.readline() @@ -318,11 +343,20 @@ class SolidLanguageServerHandler: body = self._read_bytes_from_process(self.process, self.process.stdout, num_bytes) self._handle_body(body) - except (BrokenPipeError, ConnectionResetError): - pass - return self._received_shutdown + except LanguageServerTerminatedException as e: + exception = e + except (BrokenPipeError, ConnectionResetError) as e: + exception = LanguageServerTerminatedException("Language server process terminated while reading stdout", cause=e) + except Exception as e: + exception = LanguageServerTerminatedException("Unexpected error while reading stdout from language server process", cause=e) + log.info("Language server stdout reader thread has terminated") + if not self._is_shutting_down: + if exception is None: + exception = LanguageServerTerminatedException("Language server stdout read process terminated unexpectedly") + log.error(str(exception)) + self._cancel_pending_requests(exception) - def run_forever_stderr(self) -> None: + def _read_ls_process_stderr(self) -> None: """ Continuously read from the language server process stderr and log the messages """ @@ -334,6 +368,7 @@ class SolidLanguageServerHandler: self._log("LSP stderr: " + line.decode(ENCODING, errors="replace")) except (BrokenPipeError, ConnectionResetError): pass + log.info("Language server stderr reader thread has terminated") def _handle_body(self, body: bytes) -> None: """ @@ -386,30 +421,40 @@ class SolidLanguageServerHandler: # Use lock to prevent race conditions on tasks and task_counter self._send_payload(make_error_response(request_id, err)) + def _cancel_pending_requests(self, exception: Exception) -> None: + """ + Cancel all pending requests by setting their results to an error + """ + with self._response_handlers_lock: + log.info("Cancelling %d pending language server requests", len(self._pending_requests)) + for request in self._pending_requests.values(): + log.info("Cancelling %s", request) + request.on_error(exception) + self._pending_requests.clear() + def send_request(self, method: str, params: dict | None = None) -> PayloadLike: """ Send request to the server, register the request id, and wait for the response """ - request = Request() - - # Use lock to prevent race conditions on request_id and _response_handlers with self._request_id_lock: request_id = self.request_id self.request_id += 1 + request = Request(request_id=request_id, method=method) + log.debug("Starting: %s", request) + with self._response_handlers_lock: - self._response_handlers[request_id] = request + self._pending_requests[request_id] = request self._send_payload(make_request(method, request_id, params)) self._log(f"Waiting for response to request {method} with params:\n{params}") result = request.get_result(timeout=self._request_timeout) + log.debug("Completed: %s", request) self._log("Processing result") if result.is_error(): - raise LanguageServerException( - f"Could not process request {method} with params:\n{params}.\n Language server error: {result.error}" - ) from result.error + raise LanguageServerException(f"Error processing request {method} with params:\n{params}", cause=result.error) from result.error self._log(f"Returning non-error result, which is:\n{result.payload}") return result.payload @@ -451,7 +496,7 @@ class SolidLanguageServerHandler: Handle the response received from the server for a request, using the id to determine the request """ with self._response_handlers_lock: - request = self._response_handlers.pop(response["id"]) + request = self._pending_requests.pop(response["id"]) if "result" in response and "error" not in response: request.on_result(response["result"]) @@ -499,7 +544,7 @@ class SolidLanguageServerHandler: except asyncio.CancelledError: return except Exception as ex: - if (not self._received_shutdown) and self.logger: + if (not self._is_shutting_down) and self.logger: self.logger( "client", "logger", diff --git a/src/solidlsp/ls_request.py b/src/solidlsp/ls_request.py index f81727d4..cacd2872 100644 --- a/src/solidlsp/ls_request.py +++ b/src/solidlsp/ls_request.py @@ -1,11 +1,13 @@ -from typing import Any, Union +from typing import TYPE_CHECKING, Any, Union -from solidlsp.ls_handler import SolidLanguageServerHandler from solidlsp.lsp_protocol_handler import lsp_types +if TYPE_CHECKING: + from .ls_handler import SolidLanguageServerHandler + class LanguageServerRequest: - def __init__(self, handler: SolidLanguageServerHandler): + def __init__(self, handler: "SolidLanguageServerHandler"): self.handler = handler def _send_request(self, method: str, params: Any | None = None) -> Any: