include stream output in CellExecutionError (#282)

* include captured stream output in CellExecutionError stream output may be useful for diagnosis, and does not appear to be captured for review elsewhere * Unicode check doesn't pass on Windows skip it for now, because it's not part of this PR * strip ansi codes before checking traceback contents --------- Co-authored-by: David Brochart <[email protected]>
jupyter · Apr 19, 2023 · 314c12e · 314c12e
1 parent eab9611
commit 314c12e
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 9 deletions.
diff --git a/nbclient/client.py b/nbclient/client.py
@@ -1017,7 +1017,6 @@ async def async_execute_cell(
         await run_hook(
             self.on_cell_executed, cell=cell, cell_index=cell_index, execute_reply=exec_reply
         )
-        await self._check_raise_for_error(cell, cell_index, exec_reply)
 
         if self.coalesce_streams and cell.outputs:
             new_outputs = []
@@ -1056,6 +1055,8 @@ async def async_execute_cell(
 
             cell.outputs = new_outputs
 
+        await self._check_raise_for_error(cell, cell_index, exec_reply)
+
         self.nb['cells'][cell_index] = cell
         return cell
 

diff --git a/nbclient/exceptions.py b/nbclient/exceptions.py
@@ -1,5 +1,5 @@
 """Exceptions for nbclient."""
-from typing import Dict
+from typing import Dict, List
 
 from nbformat import NotebookNode
 
@@ -84,22 +84,43 @@ def from_cell_and_msg(cls, cell: NotebookNode, msg: Dict) -> "CellExecutionError
         """Instantiate from a code cell object and a message contents
         (message is either execute_reply or error)
         """
+
+        # collect stream outputs for our error message
+        stream_outputs: List[str] = []
+        for output in cell.outputs:
+            if output["output_type"] == "stream":
+                stream_outputs.append(
+                    stream_output_msg.format(name=output["name"], text=output["text"].rstrip())
+                )
+        if stream_outputs:
+            # add blank line before, trailing separator
+            # if there is any stream output to display
+            stream_outputs.insert(0, "")
+            stream_outputs.append("------------------")
+        stream_output: str = "\n".join(stream_outputs)
+
         tb = '\n'.join(msg.get('traceback', []) or [])
         return cls(
             exec_err_msg.format(
                 cell=cell,
+                stream_output=stream_output,
                 traceback=tb,
             ),
             ename=msg.get('ename', '<Error>'),
             evalue=msg.get('evalue', ''),
         )
 
 
+stream_output_msg: str = """\
+----- {name} -----
+{text}"""
+
 exec_err_msg: str = """\
 An error occurred while executing the following cell:
 ------------------
 {cell.source}
 ------------------
+{stream_output}
 
 {traceback}
 """

diff --git a/nbclient/tests/files/Skip Exceptions with Cell Tags.ipynb b/nbclient/tests/files/Skip Exceptions with Cell Tags.ipynb
@@ -9,19 +9,36 @@
     ]
    },
    "outputs": [
+    {
+     "name": "stdout",
+     "output_type": "stream",
+     "text": [
+      "hello\n"
+     ]
+    },
+    {
+     "name": "stderr",
+     "output_type": "stream",
+     "text": [
+      "errorred\n"
+     ]
+    },
     {
      "ename": "Exception",
      "evalue": "message",
      "output_type": "error",
      "traceback": [
       "\u001b[0;31m---------------------------------------------------------------------------\u001b[0m",
       "\u001b[0;31mException\u001b[0m                                 Traceback (most recent call last)",
-      "\u001b[0;32m<ipython-input-1-644b5753a261>\u001b[0m in \u001b[0;36m<module>\u001b[0;34m\u001b[0m\n\u001b[1;32m      1\u001b[0m \u001b[0;31m# üñîçø∂é\u001b[0m\u001b[0;34m\u001b[0m\u001b[0;34m\u001b[0m\u001b[0m\n\u001b[0;32m----> 2\u001b[0;31m \u001b[0;32mraise\u001b[0m \u001b[0mException\u001b[0m\u001b[0;34m(\u001b[0m\u001b[0;34m\"message\"\u001b[0m\u001b[0;34m)\u001b[0m\u001b[0;34m\u001b[0m\u001b[0m\n\u001b[0m",
+      "Cell \u001b[0;32mIn[1], line 5\u001b[0m\n\u001b[1;32m      3\u001b[0m \u001b[38;5;28mprint\u001b[39m(\u001b[38;5;124m\"\u001b[39m\u001b[38;5;124merrorred\u001b[39m\u001b[38;5;124m\"\u001b[39m, file\u001b[38;5;241m=\u001b[39msys\u001b[38;5;241m.\u001b[39mstderr)\n\u001b[1;32m      4\u001b[0m \u001b[38;5;66;03m# üñîçø∂é\u001b[39;00m\n\u001b[0;32m----> 5\u001b[0m \u001b[38;5;28;01mraise\u001b[39;00m \u001b[38;5;167;01mException\u001b[39;00m(\u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mmessage\u001b[39m\u001b[38;5;124m\"\u001b[39m)\n",
       "\u001b[0;31mException\u001b[0m: message"
      ]
     }
    ],
    "source": [
+    "import sys\n",
+    "print(\"hello\")\n",
+    "print(\"errorred\", file=sys.stderr)\n",
     "# üñîçø∂é\n",
     "raise Exception(\"message\")"
    ]
@@ -44,7 +61,32 @@
    ]
   }
  ],
- "metadata": {},
+ "metadata": {
+  "kernelspec": {
+   "display_name": "Python 3 (ipykernel)",
+   "language": "python",
+   "name": "python3"
+  },
+  "language_info": {
+   "codemirror_mode": {
+    "name": "ipython",
+    "version": 3
+   },
+   "file_extension": ".py",
+   "mimetype": "text/x-python",
+   "name": "python",
+   "nbconvert_exporter": "python",
+   "pygments_lexer": "ipython3",
+   "version": "3.10.9"
+  },
+  "widgets": {
+   "application/vnd.jupyter.widget-state+json": {
+    "state": {},
+    "version_major": 2,
+    "version_minor": 0
+   }
+  }
+ },
  "nbformat": 4,
- "nbformat_minor": 1
+ "nbformat_minor": 4
 }
diff --git a/nbclient/tests/test_client.py b/nbclient/tests/test_client.py
@@ -5,6 +5,7 @@
 import functools
 import os
 import re
+import sys
 import threading
 import warnings
 from base64 import b64decode, b64encode
@@ -708,8 +709,13 @@ def test_allow_errors(self):
         res['metadata']['path'] = os.path.dirname(filename)
         with pytest.raises(CellExecutionError) as exc:
             run_notebook(filename, {"allow_errors": False}, res)
-            self.assertIsInstance(str(exc.value), str)
-            assert "# üñîçø∂é" in str(exc.value)
+
+        assert isinstance(str(exc.value), str)
+        exc_str = strip_ansi(str(exc.value))
+        # FIXME: we seem to have an encoding problem on Windows
+        # same check in force_raise_errors
+        if not sys.platform.startswith("win"):
+            assert "# üñîçø∂é" in exc_str
 
     def test_force_raise_errors(self):
         """
@@ -721,8 +727,23 @@ def test_force_raise_errors(self):
         res['metadata']['path'] = os.path.dirname(filename)
         with pytest.raises(CellExecutionError) as exc:
             run_notebook(filename, {"force_raise_errors": True}, res)
-            self.assertIsInstance(str(exc.value), str)
-            assert "# üñîçø∂é" in str(exc.value)
+
+        # verify CellExecutionError contents
+        exc_str = strip_ansi(str(exc.value))
+        # print for better debugging with captured output
+        # print(exc_str)
+        assert "Exception: message" in exc_str
+        # FIXME: unicode handling seems to have a problem on Windows
+        # same check in allow_errors
+        if not sys.platform.startswith("win"):
+            assert "# üñîçø∂é" in exc_str
+        assert "stderr" in exc_str
+        assert "stdout" in exc_str
+        assert "hello\n" in exc_str
+        assert "errorred\n" in exc_str
+        # stricter check for stream output format
+        assert "\n".join(["", "----- stdout -----", "hello", "---"]) in exc_str
+        assert "\n".join(["", "----- stderr -----", "errorred", "---"]) in exc_str
 
     def test_reset_kernel_client(self):
         filename = os.path.join(current_dir, 'files', 'HelloWorld.ipynb')