Better error message when starting kernel for session. (#1478)

Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
Co-authored-by: David Brochart <david.brochart@gmail.com>

Author: 3 people

examples/simple/simple_ext1/handlers.py

@@ -18,7 +18,8 @@ def get(self):
         self.log.info(f"Extension Name in {self.name} Default Handler: {self.name}")
         # A method for getting the url to static files (prefixed with /static/<name>).
         self.log.info(
-            "Static URL for / in simple_ext1 Default Handler: {}".format(self.static_url(path="/"))
+            "Static URL for / in simple_ext1 Default Handler: %s",
+            self.static_url(path="/"),
         )
         self.write("<h1>Hello Simple 1 - I am the default...</h1>")
         self.write(f"Config in {self.name} Default Handler: {self.config}")

jupyter_server/gateway/managers.py

@@ -632,9 +632,10 @@ async def get_msg(self, *args: Any, **kwargs: Any) -> dict[str, Any]:
         timeout = kwargs.get("timeout", 1)
         msg = await self._async_get(timeout=timeout)
         self.log.debug(
-            "Received message on channel: {}, msg_id: {}, msg_type: {}".format(
-                self.channel_name, msg["msg_id"], msg["msg_type"] if msg else "null"
-            )
+            "Received message on channel: %s, msg_id: %s, msg_type: %s",
+            self.channel_name,
+            msg["msg_id"],
+            msg["msg_type"] if msg else "null",
         )
         self.task_done()
         return cast("dict[str, Any]", msg)
@@ -643,9 +644,10 @@ def send(self, msg: dict[str, Any]) -> None:
         """Send a message to the queue."""
         message = json.dumps(msg, default=ChannelQueue.serialize_datetime).replace("</", "<\\/")
         self.log.debug(
-            "Sending message on channel: {}, msg_id: {}, msg_type: {}".format(
-                self.channel_name, msg["msg_id"], msg["msg_type"] if msg else "null"
-            )
+            "Sending message on channel: %s, msg_id: %s, msg_type: %s",
+            self.channel_name,
+            msg["msg_id"],
+            msg["msg_type"] if msg else "null",
         )
         self.channel_socket.send(message)
 

jupyter_server/services/contents/handlers.py

@@ -210,10 +210,9 @@ async def patch(self, path=""):
     async def _copy(self, copy_from, copy_to=None):
         """Copy a file, optionally specifying a target directory."""
         self.log.info(
-            "Copying {copy_from} to {copy_to}".format(
-                copy_from=copy_from,
-                copy_to=copy_to or "",
-            )
+            "Copying %r to %r",
+            copy_from,
+            copy_to or "",
         )
         model = await ensure_async(self.contents_manager.copy(copy_from, copy_to))
         self.set_status(201)

jupyter_server/services/sessions/handlers.py

@@ -161,14 +161,30 @@ async def patch(self, session_id):
                 changes["kernel_id"] = kernel_id
             elif model["kernel"].get("name") is not None:
                 kernel_name = model["kernel"]["name"]
-                kernel_id = await sm.start_kernel_for_session(
-                    session_id,
-                    kernel_name=kernel_name,
-                    name=before["name"],
-                    path=before["path"],
-                    type=before["type"],
-                )
-                changes["kernel_id"] = kernel_id
+
+                try:
+                    kernel_id = await sm.start_kernel_for_session(
+                        session_id,
+                        kernel_name=kernel_name,
+                        name=before["name"],
+                        path=before["path"],
+                        type=before["type"],
+                    )
+                    changes["kernel_id"] = kernel_id
+                except Exception as e:
+                    # the error message may contain sensitive information, so we want to
+                    # be careful with it, thus we only give the short repr of the exception
+                    # and the full traceback.
+                    # this should be fine as we are exposing here the same info as when we start a new kernel
+                    msg = "The '%s' kernel could not be started: %s" % (
+                        kernel_name,
+                        repr(str(e)),
+                    )
+                    status_msg = "Error starting kernel %s" % kernel_name
+                    self.log.error("Error starting kernel: %s", kernel_name)
+                    self.set_status(501)
+                    self.finish(json.dumps({"message": msg, "short_message": status_msg}))
+                    return
 
         await sm.update_session(session_id, **changes)
         s_model = await sm.get_session(session_id=session_id)

pyproject.toml

@@ -142,6 +142,7 @@ extend-select = [
   "B",  # flake8-bugbear
   "I",  # isort
   "UP",  # pyupgrade
+  "G001", # no % or f formatting in logs, prevents sttructured logging
 ]
 unfixable = [
   # Don't touch print statements