Merge pull request #1001 from oraios/fix-trace-logger

Fix logging of language server process communication, renaming

src/solidlsp/ls.py

@@ -24,7 +24,7 @@ from serena.util.file_system import match_path
 from solidlsp import ls_types
 from solidlsp.ls_config import Language, LanguageServerConfig
 from solidlsp.ls_exceptions import SolidLSPException
-from solidlsp.ls_handler import SolidLanguageServerHandler
+from solidlsp.ls_process import LanguageServerProcess
 from solidlsp.ls_types import UnifiedSymbolInformation
 from solidlsp.ls_utils import FileUtils, PathUtils, TextUtils
 from solidlsp.lsp_protocol_handler import lsp_types
@@ -470,7 +470,7 @@ class SolidLanguageServer(ABC):
             self._dependency_provider = self._create_dependency_provider()
             process_launch_info = self._create_process_launch_info()
         log.debug(f"Creating language server instance with {language_id=} and process launch info: {process_launch_info}")
-        self.server = SolidLanguageServerHandler(
+        self.server = LanguageServerProcess(
             process_launch_info,
             language=self.language,
             determine_log_level=self._determine_log_level,
@@ -2152,7 +2152,7 @@ class SolidLanguageServer(ABC):
         return self
 
     @property
-    def handler(self) -> SolidLanguageServerHandler:
+    def handler(self) -> LanguageServerProcess:
         """Access the underlying language server handler.
 
         Useful for advanced operations like sending custom commands

src/solidlsp/ls_exceptions.py

@@ -24,7 +24,7 @@ class SolidLSPException(Exception):
         :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
+        from .ls_process import LanguageServerTerminatedException
 
         return isinstance(self.cause, LanguageServerTerminatedException)
 
@@ -32,7 +32,7 @@ class SolidLSPException(Exception):
         """
         :return: the affected language for the case where the exception is caused by the language server having terminated
         """
-        from .ls_handler import LanguageServerTerminatedException
+        from .ls_process import LanguageServerTerminatedException
 
         if isinstance(self.cause, LanguageServerTerminatedException):
             return self.cause.language

src/solidlsp/ls_process.py

@@ -22,7 +22,6 @@ from solidlsp.lsp_protocol_handler.lsp_types import ErrorCodes
 from solidlsp.lsp_protocol_handler.server import (
     ENCODING,
     LSPError,
-    MessageType,
     PayloadLike,
     ProcessLaunchInfo,
     StringDict,
@@ -93,11 +92,10 @@ class Request(ToStringMixin):
             raise e
 
 
-class SolidLanguageServerHandler:
+class LanguageServerProcess:
     """
-    This class provides the implementation of Python client for the Language Server Protocol.
-    A class that launches the language server and communicates with it
-    using the Language Server Protocol (LSP).
+    Represents a language server process and provides methods for communicating with it using the
+    Language Server Protocol (LSP).
 
     It provides methods for sending requests, responses, and notifications to the server
     and for registering handlers for requests and notifications from the server.
@@ -117,7 +115,7 @@ class SolidLanguageServerHandler:
             that handle requests from the server.
         on_notification_handlers: A dictionary that maps method names to callback functions
             that handle notifications from the server.
-        logger: An optional function that takes two strings (source and destination) and
+        _trace_log_fn: An optional function that takes two strings (source and destination) and
             a payload dictionary, and logs the communication between the client and the server.
         tasks: A dictionary that maps task ids to asyncio.Task objects that represent
             the asynchronous tasks created by the handler.
@@ -152,7 +150,7 @@ class SolidLanguageServerHandler:
         self._pending_requests: dict[Any, Request] = {}
         self.on_request_handlers: dict[str, Callable[[Any], Any]] = {}
         self.on_notification_handlers: dict[str, Callable[[Any], None]] = {}
-        self.logger = logger
+        self._trace_log_fn = logger
         self.tasks: dict[int, Any] = {}
         self.task_counter = 0
         self.loop = None
@@ -301,19 +299,19 @@ 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")
+        log.info("Sending shutdown request to server")
         self.send.shutdown()
-        self._log("Received shutdown response from server")
-        self._log("Sending exit notification to server")
+        log.info("Received shutdown response from server")
+        log.info("Sending exit notification to server")
         self.notify.exit()
-        self._log("Sent exit notification to server")
+        log.info("Sent exit notification to server")
 
-    def _log(self, message: str | StringDict) -> None:
+    def _trace(self, src: str, dest: str, message: str | StringDict) -> None:
         """
-        Create a log message
+        Traces LS communication by logging the message with the source and destination of the message
         """
-        if self.logger is not None:
-            self.logger("client", "logger", message)
+        if self._trace_log_fn is not None:
+            self._trace_log_fn(src, dest, message)
 
     def _read_bytes_from_process(self, process, stream, num_bytes) -> bytes:  # type: ignore
         """Read exactly num_bytes from process stdout"""
@@ -402,18 +400,17 @@ class SolidLanguageServerHandler:
         try:
             self._receive_payload(json.loads(body))
         except OSError as ex:
-            self._log(f"malformed {ENCODING}: {ex}")
+            log.error(f"Error processing payload: {ex}", exc_info=ex)
         except UnicodeDecodeError as ex:
-            self._log(f"malformed {ENCODING}: {ex}")
+            log.error(f"Decoding error for encoding={ENCODING}: {ex}")
         except json.JSONDecodeError as ex:
-            self._log(f"malformed JSON: {ex}")
+            log.error(f"JSON decoding error: {ex}")
 
     def _receive_payload(self, payload: StringDict) -> None:
         """
         Determine if the payload received from server is for a request, response, or notification and invoke the appropriate handler
         """
-        if self.logger:
-            self.logger("server", "client", payload)
+        self._trace("ls", "solidlsp", payload)
         try:
             if "method" in payload:
                 if "id" in payload:
@@ -423,9 +420,9 @@ class SolidLanguageServerHandler:
             elif "id" in payload:
                 self._response_handler(payload)
             else:
-                self._log(f"Unknown payload type: {payload}")
+                log.error(f"Unknown payload type: {payload}")
         except Exception as err:
-            self._log(f"Error handling server payload: {err}")
+            log.error(f"Error handling server payload: {err}")
 
     def send_notification(self, method: str, params: dict | None = None) -> None:
         """
@@ -443,7 +440,6 @@ class SolidLanguageServerHandler:
         """
         Send error response to the given request id to the server with the given error
         """
-        # 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:
@@ -473,15 +469,14 @@ class SolidLanguageServerHandler:
 
         self._send_payload(make_request(method, request_id, params))
 
-        self._log(f"Waiting for response to request {method} with params:\n{params}")
+        log.debug("Waiting for response to request %s with params:\n%s", method, params)
         result = request.get_result(timeout=self._request_timeout)
         log.debug("Completed: %s", request)
 
-        self._log("Processing result")
         if result.is_error():
             raise SolidLSPException(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}")
+        log.debug("Returning result:\n%s", result.payload)
         return result.payload
 
     def _send_payload(self, payload: StringDict) -> None:
@@ -490,7 +485,7 @@ class SolidLanguageServerHandler:
         """
         if not self.process or not self.process.stdin:
             return
-        self._log(payload)
+        self._trace("solidlsp", "ls", payload)
         msg = create_message(payload)
 
         # Use lock to prevent concurrent writes to stdin that cause buffer corruption
@@ -500,8 +495,7 @@ class SolidLanguageServerHandler:
                 self.process.stdin.flush()
             except (BrokenPipeError, ConnectionResetError, OSError) as e:
                 # Log the error but don't raise to prevent cascading failures
-                if self.logger:
-                    self.logger("client", "logger", f"Failed to write to stdin: {e}")
+                log.error(f"Failed to write to stdin: {e}")
                 return
 
     def on_request(self, method: str, cb: Callable[[Any], Any]) -> None:
@@ -569,23 +563,12 @@ class SolidLanguageServerHandler:
         params = response.get("params")
         handler = self.on_notification_handlers.get(method)
         if not handler:
-            self._log(f"unhandled {method}")
+            log.warning("Unhandled method '%s'", method)
             return
         try:
             handler(params)
         except asyncio.CancelledError:
             return
         except Exception as ex:
-            if (not self._is_shutting_down) and self.logger:
-                self.logger(
-                    "client",
-                    "logger",
-                    str(
-                        {
-                            "type": MessageType.error,
-                            "message": str(ex),
-                            "method": method,
-                            "params": params,
-                        }
-                    ),
-                )
+            if not self._is_shutting_down:
+                log.error("Error handling notification for method '%s': %s", method, ex, exc_info=ex)

src/solidlsp/ls_request.py

@@ -3,11 +3,11 @@ from typing import TYPE_CHECKING, Any, Union
 from solidlsp.lsp_protocol_handler import lsp_types
 
 if TYPE_CHECKING:
-    from .ls_handler import SolidLanguageServerHandler
+    from .ls_process import LanguageServerProcess
 
 
 class LanguageServerRequest:
-    def __init__(self, handler: "SolidLanguageServerHandler"):
+    def __init__(self, handler: "LanguageServerProcess"):
         self.handler = handler
 
     def _send_request(self, method: str, params: Any | None = None) -> Any: