raise PapermillExecutionError when CellExecutionError is raised witho…

…ut cell error output (#786)

* raise PapermillExecutionError when CellExecutionError is raised without cell error output

* handle SystemExit

* update deprecated pytest usage

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* unit test for handling magic command error

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

papermill/execute.py

@@ -196,22 +196,35 @@ def raise_for_execution_errors(nb, output_path):
     """
     error = None
     for index, cell in enumerate(nb.cells):
-        if cell.get("outputs") is None:
-            continue
-
-        for output in cell.outputs:
-            if output.output_type == "error":
-                if output.ename == "SystemExit" and (output.evalue == "" or output.evalue == "0"):
-                    continue
-                error = PapermillExecutionError(
-                    cell_index=index,
-                    exec_count=cell.execution_count,
-                    source=cell.source,
-                    ename=output.ename,
-                    evalue=output.evalue,
-                    traceback=output.traceback,
-                )
-                break
+        has_sys_exit = False
+        # check if there is any cell error output
+        if "outputs" in cell:
+            for output in cell.outputs:
+                if output.output_type == "error":
+                    if output.ename == "SystemExit" and (output.evalue == "" or output.evalue == "0"):
+                        has_sys_exit = True
+                        continue
+                    error = PapermillExecutionError(
+                        cell_index=index,
+                        exec_count=cell.execution_count,
+                        source=cell.source,
+                        ename=output.ename,
+                        evalue=output.evalue,
+                        traceback=output.traceback,
+                    )
+                    break
+
+        # handle the CellExecutionError exceptions raised that didn't produce a cell error output
+        if not has_sys_exit and cell.get("metadata", {}).get("papermill", {}).get("exception") is True:
+            error = PapermillExecutionError(
+                cell_index=index,
+                exec_count=cell.execution_count,
+                source=cell.source,
+                ename="CellExecutionError",
+                evalue="",
+                traceback="",
+            )
+            break
 
     if error:
         # Write notebook back out with the Error Message at the top of the Notebook, and a link to

papermill/tests/notebooks/line_magic_error.ipynb

@@ -0,0 +1,43 @@
+{
+ "cells": [
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "%undefined-line-magic"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "print(\"Cell should not execute.\")"
+   ]
+  }
+ ],
+ "metadata": {
+  "kernelspec": {
+   "display_name": "Python 3",
+   "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.6.7"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 2
+}

papermill/tests/test_execute.py

@@ -327,6 +327,19 @@ def test_system_exit(self):
         self.assertEqual(nb.cells[1].outputs[0].evalue, '')
         self.assertEqual(nb.cells[2].execution_count, None)
 
+    def test_line_magic_error(self):
+        notebook_name = 'line_magic_error.ipynb'
+        result_path = os.path.join(self.test_dir, f'output_{notebook_name}')
+        with self.assertRaises(PapermillExecutionError):
+            execute_notebook(get_notebook_path(notebook_name), result_path)
+        nb = load_notebook_node(result_path)
+        self.assertEqual(nb.cells[0].cell_type, "markdown")
+        self.assertRegex(nb.cells[0].source, r'^<span .*<a href="#papermill-error-cell".*In \[1\].*</span>$')
+        self.assertEqual(nb.cells[0].metadata["tags"], ["papermill-error-cell-tag"])
+        self.assertEqual(nb.cells[2].cell_type, "code")
+        self.assertEqual(nb.cells[2].execution_count, 1)
+        self.assertEqual(nb.cells[3].execution_count, None)
+
 
 class TestNotebookValidation(unittest.TestCase):
     def setUp(self):

papermill/tests/test_iorw.py

@@ -2,6 +2,7 @@
 import json
 import os
 import unittest
+import warnings
 from tempfile import TemporaryDirectory
 from unittest.mock import Mock, patch
 
@@ -137,9 +138,9 @@ def test_read_with_invalid_file_extension(self):
             self.papermill_io.read("fake/path/fakeinputpath.ipynb1")
 
     def test_read_with_valid_file_extension(self):
-        with pytest.warns(None) as warns:
+        with warnings.catch_warnings():
+            warnings.simplefilter("error")
             self.papermill_io.read("fake/path/fakeinputpath.ipynb")
-        self.assertEqual(len(warns), 0)
 
     def test_read_yaml_with_no_file_extension(self):
         with pytest.warns(UserWarning):