Report build errors to the client

lib/esbonio/esbonio/server/__init__.py

@@ -3,11 +3,13 @@
 from ._uri import Uri
 from .feature import LanguageFeature
 from .server import EsbonioLanguageServer
+from .server import EsbonioWorkspace
 
 __all__ = [
     "DIRECTIVE",
     "ROLE",
     "EsbonioLanguageServer",
+    "EsbonioWorkspace",
     "LanguageFeature",
     "Uri",
 ]

lib/esbonio/esbonio/server/features/sphinx_manager/__init__.py

@@ -3,11 +3,19 @@
 from esbonio.server import EsbonioLanguageServer
 
 from .client import SphinxClient
+from .client_mock import MockSphinxClient
+from .client_mock import mock_sphinx_client_factory
 from .client_subprocess import make_subprocess_sphinx_client
 from .config import SphinxConfig
 from .manager import SphinxManager
 
-__all__ = ["SphinxClient", "SphinxConfig", "SphinxManager"]
+__all__ = [
+    "SphinxClient",
+    "SphinxConfig",
+    "SphinxManager",
+    "MockSphinxClient",
+    "mock_sphinx_client_factory",
+]
 
 
 def esbonio_setup(server: EsbonioLanguageServer):

lib/esbonio/esbonio/server/features/sphinx_manager/client_mock.py

@@ -0,0 +1,84 @@
+from __future__ import annotations
+
+import asyncio
+import typing
+from typing import Dict
+from typing import Optional
+from typing import Union
+
+from esbonio.sphinx_agent import types
+
+if typing.TYPE_CHECKING:
+    from esbonio.server import Uri
+
+    from .client import SphinxClient
+    from .manager import SphinxManager
+
+
+class MockSphinxClient:
+    """A mock SphinxClient implementation, used for testing."""
+
+    def __init__(
+        self,
+        create_result: Union[types.SphinxInfo, Exception],
+        build_result: Optional[Union[types.BuildResult, Exception]] = None,
+        build_file_map: Optional[Dict[Uri, str]] = None,
+        startup_delay: float = 0.5,
+    ):
+        """Create a new instance.
+
+        Parameters
+        ----------
+        create_result
+           The result to return when calling ``create_application``.
+           If an exception is given it will be raised.
+
+        build_result
+           The result to return when calling ``build``.
+           If an exception is given it will be raised.
+
+        build_file_map
+           The build file map to use.
+
+        startup_delay
+           The duration to sleep for when calling ``start``
+        """
+        self._create_result = create_result
+        self._startup_delay = startup_delay
+        self._build_result = build_result or types.BuildResult()
+
+        self.building = False
+        self.build_file_map = build_file_map or {}
+
+    async def start(self, *args, **kwargs):
+        await asyncio.sleep(self._startup_delay)
+
+    async def stop(self, *args, **kwargs):
+        pass
+
+    async def create_application(self, *args, **kwargs) -> types.SphinxInfo:
+        """Create an application."""
+
+        if isinstance(self._create_result, Exception):
+            raise self._create_result
+
+        return self._create_result
+
+    async def build(self, *args, **kwargs) -> types.BuildResult:
+        """Trigger a build"""
+
+        if isinstance(self._build_result, Exception):
+            raise self._build_result
+
+        return self._build_result
+
+
+def mock_sphinx_client_factory(client: Optional[SphinxClient] = None):
+    """Return a factory function that can be used with a ``SphinxManager`` instance."""
+
+    def factory(manager: SphinxManager):
+        if client is None:
+            raise RuntimeError("Unexpected client creation")
+        return client
+
+    return factory

lib/esbonio/esbonio/server/features/sphinx_manager/client_subprocess.py

@@ -187,9 +187,10 @@ async def build(
         )
 
         self._building = True
-
-        result = await self.protocol.send_request_async("sphinx/build", params)
-        self._building = False
+        try:
+            result = await self.protocol.send_request_async("sphinx/build", params)
+        finally:
+            self._building = False
 
         self._diagnostics = {
             Uri.for_file(fpath): items for fpath, items in result.diagnostics.items()

lib/esbonio/esbonio/server/features/sphinx_manager/manager.py

@@ -105,7 +105,11 @@ async def trigger_build(self, uri: Uri):
             if saved_version < doc_version and (fs_path := src_uri.fs_path) is not None:
                 content_overrides[fs_path] = doc.source
 
-        result = await client.build(content_overrides=content_overrides)
+        try:
+            result = await client.build(content_overrides=content_overrides)
+        except Exception as exc:
+            self.server.show_message(f"{exc}", lsp.MessageType.Error)
+            return
 
         # Update diagnostics
         source = f"sphinx[{client.id}]"
@@ -164,7 +168,11 @@ async def get_client(self, uri: Uri) -> Optional[SphinxClient]:
 
         # Create a new client instance.
         self._client_creating = asyncio.create_task(self._create_client(uri))
-        return await self._client_creating
+        try:
+            return await self._client_creating
+        finally:
+            # Be sure to unset the task when it resolves
+            self._client_creating = None
 
     async def _create_client(self, uri: Uri) -> Optional[SphinxClient]:
         """Create a new sphinx client instance."""
@@ -181,7 +189,14 @@ async def _create_client(self, uri: Uri) -> Optional[SphinxClient]:
         client = self.client_factory(self)
         await client.start(resolved)
 
-        sphinx_info = await client.create_application(resolved)
+        try:
+            sphinx_info = await client.create_application(resolved)
+        except Exception as exc:
+            self.logger.error("Unable to create sphinx application", exc_info=True)
+            self.server.show_message(f"{exc}", lsp.MessageType.Error)
+
+            await client.stop()
+            return None
 
         if client.src_uri is None:
             self.logger.error("No src uri!")

lib/esbonio/esbonio/server/server.py

@@ -50,23 +50,23 @@ class EsbonioWorkspace(Workspace):
 
     def get_document(self, doc_uri: str) -> Document:
         uri = str(Uri.parse(doc_uri).resolve())
-        return super().get_document(uri)
+        return super().get_text_document(uri)
 
     def put_document(self, text_document: types.TextDocumentItem):
         text_document.uri = str(Uri.parse(text_document.uri).resolve())
-        return super().put_document(text_document)
+        return super().put_text_document(text_document)
 
     def remove_document(self, doc_uri: str):
         doc_uri = str(Uri.parse(doc_uri).resolve())
-        return super().remove_document(doc_uri)
+        return super().remove_text_document(doc_uri)
 
     def update_document(
         self,
         text_doc: types.VersionedTextDocumentIdentifier,
         change: types.TextDocumentContentChangeEvent,
     ):
         text_doc.uri = str(Uri.parse(text_doc.uri).resolve())
-        return super().update_document(text_doc, change)
+        return super().update_text_document(text_doc, change)
 
 
 class EsbonioLanguageServer(LanguageServer):

lib/esbonio/esbonio/sphinx_agent/handlers.py

@@ -3,6 +3,7 @@
 import os.path
 import pathlib
 import sys
+import traceback
 import typing
 from functools import partial
 from typing import IO
@@ -29,6 +30,7 @@
 from .util import send_message
 
 STATIC_DIR = (pathlib.Path(__file__).parent / "static").resolve()
+sphinx_logger = logging.getLogger(SPHINX_LOG_NAMESPACE)
 
 
 class SphinxHandler:
@@ -173,8 +175,6 @@ def setup_logging(self, config: SphinxConfig, app: Sphinx, status: IO, warning:
         console.nocolor()
 
         if not config.silent:
-            sphinx_logger = logging.getLogger(SPHINX_LOG_NAMESPACE)
-
             # Be sure to remove any old handlers
             for handler in sphinx_logger.handlers:
                 if isinstance(handler, SphinxLogHandler):
@@ -226,8 +226,12 @@ def build_sphinx_app(self, request: types.BuildRequest):
                 jsonrpc=request.jsonrpc,
             )
             send_message(response)
-        except Exception:
-            send_error(id=request.id, code=-32602, message="Sphinx build failed.")
+        except Exception as exc:
+            message = "".join(traceback.format_exception_only(type(exc), exc))
+            sphinx_logger.error("sphinx-build failed", exc_info=True)
+            send_error(
+                id=request.id, code=-32603, message=f"sphinx-build failed: {message}"
+            )
 
     def notify_exit(self, request: types.ExitNotification):
         """Sent from the client to signal that the agent should exit."""

lib/esbonio/esbonio/sphinx_agent/server.py

@@ -58,7 +58,7 @@ def handle_message(data: bytes):
         if msg_id is not None:
             send_error(
                 id=msg_id,
-                code=-32602,
+                code=-32603,
                 message=f"{e}",
                 data=dict(traceback=traceback.format_exc()),
             )

lib/esbonio/tests/server/features/test_sphinx_manager.py

@@ -0,0 +1,83 @@
+from unittest.mock import AsyncMock
+from unittest.mock import Mock
+
+import pytest
+from lsprotocol import types as lsp
+from pygls.exceptions import JsonRpcInternalError
+
+from esbonio.server import EsbonioLanguageServer
+from esbonio.server import EsbonioWorkspace
+from esbonio.server import Uri
+from esbonio.server.features.sphinx_manager import MockSphinxClient
+from esbonio.server.features.sphinx_manager import SphinxConfig
+from esbonio.server.features.sphinx_manager import SphinxManager
+from esbonio.server.features.sphinx_manager import mock_sphinx_client_factory
+from esbonio.sphinx_agent import types
+
+
+@pytest.fixture()
+def workspace(uri_for):
+    return uri_for("sphinx-default", "workspace")
+
+
+@pytest.fixture()
+def server(event_loop, workspace: Uri):
+    """A mock instance of the language"""
+    esbonio = EsbonioLanguageServer(loop=event_loop)
+    esbonio.show_message = Mock()
+    esbonio.get_user_config = AsyncMock(return_value=SphinxConfig())
+    esbonio.lsp._workspace = EsbonioWorkspace(
+        None,
+        workspace_folders=[lsp.WorkspaceFolder(str(workspace), "workspace")],
+    )
+    return esbonio
+
+
+@pytest.fixture()
+def sphinx_info(workspace: Uri):
+    return types.SphinxInfo(
+        id="123",
+        version="6.0",
+        conf_dir=workspace.fs_path,
+        build_dir=(workspace / "_build" / "html").fs_path,
+        builder_name="html",
+        src_dir=workspace.fs_path,
+    )
+
+
+@pytest.mark.asyncio
+async def test_create_application_error(server, workspace: Uri):
+    """Ensure that we can handle errors during application creation correctly."""
+
+    client = MockSphinxClient(JsonRpcInternalError("create sphinx application failed."))
+    client_factory = mock_sphinx_client_factory(client)
+
+    manager = SphinxManager(client_factory, server)
+
+    result = await manager.get_client(workspace / "index.rst")
+    assert result is None
+
+    server.show_message.assert_called_with(
+        "create sphinx application failed.", lsp.MessageType.Error
+    )
+
+
+@pytest.mark.asyncio
+async def test_trigger_build_error(sphinx_info, server, workspace):
+    """Ensure that we can handle build errors correctly."""
+
+    client = MockSphinxClient(
+        sphinx_info,
+        build_file_map={(workspace / "index.rst").resolve(): "/"},
+        build_result=JsonRpcInternalError("sphinx-build failed:"),
+    )
+    client_factory = mock_sphinx_client_factory()
+
+    manager = SphinxManager(client_factory, server)
+    manager.clients = {workspace: client}
+
+    await manager.trigger_build(workspace / "index.rst")
+
+    server.show_message.assert_called_with(
+        "sphinx-build failed:", lsp.MessageType.Error
+    )