From 8e2013d8c15bd11e10621c62c94c3781606ba295 Mon Sep 17 00:00:00 2001
From: Dominik Jain <dominik.jain@outlook.com>
Date: Sun, 6 Apr 2025 21:42:49 +0200
Subject: [PATCH] Add mechanism for showing fatal errors in GUI upon
 initialization failure

---
 pyproject.toml               |  1 +
 src/serena/agno.py           |  7 +++++-
 src/serena/gui_log_viewer.py | 41 +++++++++++++++++++++++++++---------
 src/serena/mcp.py            |  8 ++-----
 4 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 8b48ffa2..a6de3911 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -233,6 +233,7 @@ ignore = [
   "W293",
   "B009",
   "SIM103",  # forbids multiple returns
+  "SIM110",  # requires use of any(...) instead of for-loop
 ]
 unfixable = [
   "F841",
diff --git a/src/serena/agno.py b/src/serena/agno.py
index 09893f29..04832934 100644
--- a/src/serena/agno.py
+++ b/src/serena/agno.py
@@ -16,6 +16,7 @@ from sensai.util.logging import LogTime
 
 from serena import serena_root_path
 from serena.agent import SerenaAgent, Tool
+from serena.gui_log_viewer import show_fatal_exception
 
 log = logging.getLogger(__name__)
 
@@ -146,7 +147,11 @@ class SerenaAgnoAgentProvider:
                 project_file = None
 
             with LogTime("Loading Serena agent"):
-                serena_agent = SerenaAgent(project_file)
+                try:
+                    serena_agent = SerenaAgent(project_file)
+                except Exception as e:
+                    show_fatal_exception(e)
+                    raise
 
             # Even though we don't want to keep history between sessions,
             # for agno-ui to work as a conversation, we use a persistent storage on disk.
diff --git a/src/serena/gui_log_viewer.py b/src/serena/gui_log_viewer.py
index aa5dc518..dfe554a6 100644
--- a/src/serena/gui_log_viewer.py
+++ b/src/serena/gui_log_viewer.py
@@ -3,10 +3,14 @@ import logging
 import queue
 import sys
 import threading
+import time
 import tkinter as tk
+import traceback
 from enum import Enum, auto
 from pathlib import Path
 
+log = logging.getLogger(__name__)
+
 
 class LogLevel(Enum):
     DEBUG = auto()
@@ -57,8 +61,7 @@ class GuiLogViewer:
         """Start the log viewer in a separate thread."""
         if not self.running:
             self.print_status("Starting thread")
-            self.running = True
-            self.log_thread = threading.Thread(target=self._run_gui)
+            self.log_thread = threading.Thread(target=self.run_gui)
             self.log_thread.daemon = True
             self.log_thread.start()
             return True
@@ -67,7 +70,6 @@ class GuiLogViewer:
     def stop(self):
         """Stop the log viewer."""
         if self.running:
-            self.running = False
             # Add a sentinel value to the queue to signal the GUI to exit
             self.message_queue.put(None)
             return True
@@ -91,11 +93,7 @@ class GuiLogViewer:
             message (str): The log message to display
 
         """
-        if self.running:
-            # Add the original message so we can determine log level correctly
-            self.message_queue.put(message)
-            return True
-        return False
+        self.message_queue.put(message)
 
     def _determine_log_level(self, message):
         """
@@ -199,8 +197,9 @@ class GuiLogViewer:
             if self.running:
                 self.root.after(100, self._process_queue)
 
-    def _run_gui(self):
-        """Run the GUI in a separate thread."""
+    def run_gui(self):
+        """Run the GUI"""
+        self.running = True
         try:
             self.root = tk.Tk()
             self.root.title(self.title)
@@ -346,3 +345,25 @@ class GuiLogViewerHandler(logging.Handler):
         """
         if self.log_viewer.running:
             self.log_viewer.stop()
+
+
+def show_fatal_exception(e: Exception, duration_secs: int = 60):
+    """
+    Makes sure the given exception is shown in the GUI log viewer,
+    either an existing instance or a new one.
+
+    :param e: the exception to display
+    :param duration_secs: the duration for which to display the error before
+        terminating the program for the case where the log viewer is already present
+        in a daemon thread which will terminate when the program does
+    """
+    if GuiLogViewerHandler.is_instance_registered():
+        # show in existing daemon thread, waiting for the given duration
+        log.error(f"Fatal error: {e}", exc_info=e)
+        time.sleep(duration_secs)
+    else:
+        # show in new window in main thread (user must close it)
+        log_viewer = GuiLogViewer()
+        exc_info = "".join(traceback.format_exception(type(e), e, e.__traceback__))
+        log_viewer.add_log(f"ERROR Fatal exception: {e}\n{exc_info}")
+        log_viewer.run_gui()
diff --git a/src/serena/mcp.py b/src/serena/mcp.py
index 23003940..d55a5b1c 100644
--- a/src/serena/mcp.py
+++ b/src/serena/mcp.py
@@ -2,7 +2,6 @@
 The Serena Model Context Protocol (MCP) Server
 """
 
-import time
 import sys
 from collections.abc import AsyncIterator
 from contextlib import asynccontextmanager
@@ -17,7 +16,7 @@ from sensai.util import logging
 from sensai.util.helper import mark_used
 
 from serena.agent import SerenaAgent, Tool
-from serena.gui_log_viewer import GuiLogViewerHandler
+from serena.gui_log_viewer import show_fatal_exception
 
 log = logging.getLogger(__name__)
 LOG_FORMAT = "%(levelname)-5s %(asctime)-15s %(name)s:%(funcName)s:%(lineno)d - %(message)s"
@@ -101,10 +100,7 @@ def create_mcp_server() -> FastMCP:
             # project_activation_callback=update_tools
         )
     except Exception as e:
-        # allow any initialization errors to be viewed for some time in the GUI before exiting
-        if GuiLogViewerHandler.is_instance_registered():
-            log.error(f"Failed to initialize agent: {e}", exc_info=e)
-            time.sleep(60)
+        show_fatal_exception(e)
         raise
 
     @asynccontextmanager