Fix: Respect allow_errors value

tests/app/notebooks_test.py

@@ -1,4 +1,6 @@
 # simply tests if some notebooks execute
+import sys
+
 import pytest
 
 
@@ -8,6 +10,7 @@ def voila_args(notebook_directory, voila_args_extra):
 
 
 @pytest.mark.gen_test
+@pytest.mark.skipif(sys.version_info < (3, 6), reason="Matplotlib installation issue on MacOS, Python 3.5")
 def test_other_comms(http_client, base_url):
     response = yield http_client.fetch(base_url + '/voila/render/other_comms.ipynb')
     html_text = response.body.decode('utf-8')

voila/execute.py

@@ -18,26 +18,6 @@
 from ipykernel.jsonutil import json_clean
 
 
-def strip_code_cell_warnings(cell):
-    """Strip any warning outputs and traceback from a code cell."""
-    if cell['cell_type'] != 'code':
-        return cell
-
-    outputs = cell['outputs']
-
-    cell['outputs'] = [
-        output for output in outputs
-        if output['output_type'] != 'stream' or output['name'] != 'stderr'
-    ]
-
-    return cell
-
-
-def should_strip_error(config):
-    """Return True if errors should be stripped from the Notebook, False otherwise, depending on the current config."""
-    return 'Voila' not in config or 'log_level' not in config['Voila'] or config['Voila']['log_level'] != logging.DEBUG
-
-
 class OutputWidget:
     """This class mimics a front end output widget"""
     def __init__(self, comm_id, state, kernel_client, executor):
@@ -128,31 +108,25 @@ def __init__(self, **kwargs):
         self.output_objects = {}
 
     def preprocess(self, nb, resources, km=None):
-        try:
-            result = super(VoilaExecutePreprocessor, self).preprocess(nb, resources=resources, km=km)
-        except CellExecutionError as e:
-            self.log.error(e)
-            result = (nb, resources)
+        result = super(VoilaExecutePreprocessor, self).preprocess(nb, resources=resources, km=km)
 
-        # Strip errors and traceback if not in debug mode
-        if should_strip_error(self.config):
-            self.strip_notebook_errors(nb)
+        # Strip errors if not in debug mode
+        self.strip_notebook_errors(nb)
 
         return result
 
     def preprocess_cell(self, cell, resources, cell_index, store_history=True):
+        # TODO: pass store_history as a 5th argument when we can require nbconver >=5.6.1
+        # result = super(VoilaExecutePreprocessor, self).preprocess_cell(cell, resources, cell_index, store_history)
         try:
-            # TODO: pass store_history as a 5th argument when we can require nbconver >=5.6.1
-            # result = super(VoilaExecutePreprocessor, self).preprocess_cell(cell, resources, cell_index, store_history)
             result = super(VoilaExecutePreprocessor, self).preprocess_cell(cell, resources, cell_index)
         except CellExecutionError as e:
-            self.log.error(e)
-            result = (cell, resources)
-
-        # Strip errors and traceback if not in debug mode
-        if should_strip_error(self.config):
-            strip_code_cell_warnings(cell)
+            # Strip errors if not in debug mode
             self.strip_code_cell_errors(cell)
+            raise e
+
+        # Strip errors if not in debug mode
+        self.strip_code_cell_errors(cell)
 
         return result
 
@@ -202,27 +176,39 @@ def clear_output(self, outs, msg, cell_index):
             return
         super(VoilaExecutePreprocessor, self).clear_output(outs, msg, cell_index)
 
+    @property
+    def should_strip_error(self):
+        """Return True if errors should be stripped from the Notebook, False otherwise, depending on the current config."""
+        return 'Voila' not in self.config or 'log_level' not in self.config['Voila'] or self.config['Voila']['log_level'] != logging.DEBUG
+
     def strip_notebook_errors(self, nb):
         """Strip error messages and traceback from a Notebook."""
+        if not self.should_strip_error:
+            return nb
+
         cells = nb['cells']
 
         code_cells = [cell for cell in cells if cell['cell_type'] == 'code']
 
         for cell in code_cells:
-            strip_code_cell_warnings(cell)
             self.strip_code_cell_errors(cell)
 
         return nb
 
     def strip_code_cell_errors(self, cell):
         """Strip any error outputs and traceback from a code cell."""
         # There is no 'outputs' key for markdown cells
-        if cell['cell_type'] != 'code':
+        if not self.should_strip_error or cell['cell_type'] != 'code':
             return cell
 
-        outputs = cell['outputs']
+        # Strip warnings
+        cell['outputs'] = [
+            output for output in cell['outputs']
+            if output['output_type'] != 'stream' or output['name'] != 'stderr'
+        ]
 
-        error_outputs = [output for output in outputs if output['output_type'] == 'error']
+        # Strip errors
+        error_outputs = [output for output in cell['outputs'] if output['output_type'] == 'error']
 
         error_message = 'There was an error when executing cell [{}]. {}'.format(cell['execution_count'], self.cell_error_instruction)
 

voila/handler.py

@@ -17,6 +17,7 @@
 import nbformat
 
 from nbconvert.preprocessors import ClearOutputPreprocessor
+from nbconvert.preprocessors.execute import CellExecutionError
 
 from .execute import executenb, VoilaExecutePreprocessor
 from .exporter import VoilaExporter
@@ -121,7 +122,11 @@ def _jinja_kernel_start(self):
 
     def _jinja_notebook_execute(self, nb, kernel_id):
         km = self.kernel_manager.get_kernel(kernel_id)
-        result = executenb(nb, km=km, cwd=self.cwd, config=self.traitlet_config)
+        try:
+            result = executenb(nb, km=km, cwd=self.cwd, config=self.traitlet_config)
+        except CellExecutionError as e:
+            self.log.error(e)
+            raise tornado.web.HTTPError(500, 'There was an error executing the Notebook')
         # we modify the notebook in place, since the nb variable cannot be reassigned it seems in jinja2
         # e.g. if we do {% with nb = notebook_execute(nb, kernel_id) %}, the base template/blocks will not
         # see the updated variable (it seems to be local to our block)
@@ -136,9 +141,15 @@ def _jinja_cell_generator(self, nb, kernel_id):
 
         with ep.setup_preprocessor(nb, resources, km=km):
             for cell_idx, cell in enumerate(nb.cells):
-                res = ep.preprocess_cell(cell, resources, cell_idx, store_history=False)
-
-                yield res[0]
+                try:
+                    ep.preprocess_cell(cell, resources, cell_idx, store_history=False)
+
+                    yield cell
+                except CellExecutionError as e:
+                    # Still return the last cell, log the error, and stop the execution.
+                    yield cell
+                    self.log.error(e)
+                    break
 
     # @tornado.gen.coroutine
     async def load_notebook(self, path):