Merge pull request #12238 from quarto-dev/jupyter/display-error

Add error handling for IPython display errors in notebook cell execution

Author: cderv

news/changelog-1.7.md

@@ -140,6 +140,7 @@ All changes included in 1.7:
 
 - ([#12114](https://github.com/quarto-dev/quarto-cli/issues/12114)): `JUPYTERCACHE` environment variable from [Jupyter cache CLI](https://jupyter-cache.readthedocs.io/en/latest/using/cli.html) is now respected by Quarto when `cache: true` is used. This environment variable allows to change the path of the cache directory.
 - ([#12374](https://github.com/quarto-dev/quarto-cli/issues/12374)): Detect language properly in Jupyter notebooks that lack the `language` field in their `kernelspec`s.
+- ([#12228](https://github.com/quarto-dev/quarto-cli/issues/12228)): `quarto render` will now fails if errors are detected at IPython display level. Setting `error: true` globally or at cell level will keep the error to show in output and not stop the rendering.
 
 ## Other Fixes and Improvements
 
@@ -160,6 +161,7 @@ All changes included in 1.7:
 - ([#11803](https://github.com/quarto-dev/quarto-cli/pull/11803)): Added a new CLI command `quarto call`. First users of this interface are the new `quarto call engine julia ...` subcommands.
 - ([#12338](https://github.com/quarto-dev/quarto-cli/issues/12338)): Add an additional workaround for the SCSS parser used in color variable extraction.
 - ([#12369](https://github.com/quarto-dev/quarto-cli/pull/12369)): `quarto preview` correctly throws a YAML validation error when a `format` key does not conform.
+- ([#12238](https://gijit.com/quarto-dev/quarto-cli/issues/12238)): Very long error (e.g. in Jupyter Notenook with backtrace) are now no more truncated in the console.
 
 ## Languages
 

src/core/log.ts

@@ -37,6 +37,7 @@ export interface LogMessageOptions {
   indent?: number;
   format?: (line: string) => string;
   colorize?: boolean;
+  stripAnsiCode?: boolean;
 }
 
 // deno-lint-ignore no-explicit-any
@@ -156,9 +157,23 @@ export class StdErrOutputHandler extends BaseHandler {
     return msg;
   }
   log(msg: string): void {
-    Deno.stderr.writeSync(
-      new TextEncoder().encode(msg),
-    );
+    const encoder = new TextEncoder();
+    const data = encoder.encode(msg);
+
+    let bytesWritten = 0;
+    while (bytesWritten < data.length) {
+      // Write the remaining portion of the buffer
+      const remaining = data.subarray(bytesWritten);
+      const written = Deno.stderr.writeSync(remaining);
+
+      // If we wrote 0 bytes, something is wrong - avoid infinite loop
+      if (written === 0) {
+        // Could add fallback handling here if needed
+        break;
+      }
+
+      bytesWritten += written;
+    }
   }
 }
 
@@ -215,6 +230,7 @@ export class LogFileHandler extends FileHandler {
         ...logRecord.args[0] as LogMessageOptions,
         bold: false,
         dim: false,
+        stripAnsiCode: true,
         format: undefined,
       };
       let msg = applyMsgOptions(logRecord.msg, options);
@@ -403,7 +419,9 @@ function applyMsgOptions(msg: string, options: LogMessageOptions) {
   if (options.format) {
     msg = options.format(msg);
   }
-
+  if (options.stripAnsiCode) {
+    msg = colors.stripAnsiCode(msg);
+  }
   return msg;
 }
 

src/resources/jupyter/jupyter.py

@@ -19,6 +19,7 @@
 
 from log import log_init, log, log_error, trace
 from notebook import notebook_execute, RestartKernel
+from nbclient.exceptions import CellExecutionError
 
 import asyncio
 if sys.platform == 'win32':
@@ -226,21 +227,22 @@ def run_server_subprocess(options, status):
 # run a notebook directly (not a server)
 def run_notebook(options, status):
 
-   # run notebook w/ some special exception handling. note that we don't 
+   # run notebook w/ some special exception handling. note that we don't
    # log exceptions here b/c they are considered normal course of execution
    # for errors that occur in notebook cells
-   try:   
+   try:
+      trace('Running notebook_execute')
       notebook_execute(options, status)
    except Exception as e:
+      trace(f'run_notebook caught exception: {type(e).__name__}')
       # CellExecutionError for execution at the terminal includes a bunch
       # of extra stack frames internal to this script. remove them
       msg = str(e)
       kCellExecutionError = "nbclient.exceptions.CellExecutionError: "
       loc = msg.find(kCellExecutionError)
       if loc != -1:
-         msg = msg[loc + len(kCellExecutionError)]
-      sys.stderr.write("\n\n" + msg + "\n")
-      sys.stderr.flush()
+         msg = msg[loc + len(kCellExecutionError):]
+      status("\n\n" + msg + "\n")
       sys.exit(1)
 
 

src/resources/jupyter/log.py

@@ -37,8 +37,10 @@ def log(level, msg):
    def log(level, msg):
       logging.getLogger().log(level, msg)
 
-def log_error(msg, exc_info = False):
-   logging.getLogger().log(logging.ERROR, msg, exc_info = exc_info, stack_info = not exc_info)
+def log_error(msg, exc_info = False, stack_info = None):
+   if stack_info is None:
+      stack_info = not exc_info
+   logging.getLogger().log(logging.ERROR, msg, exc_info = exc_info, stack_info = stack_info)
 
 def trace(msg):
    prev_frame = inspect.stack()[1]

src/resources/jupyter/notebook.py

@@ -517,9 +517,11 @@ def cell_execute(client, cell, index, execution_count, eval_default, store_histo
 
    # check options for eval and error
    eval = cell_options.get('eval', eval_default)
-   allow_errors = cell_options.get('error', False)
+   allow_errors = cell_options.get('error')
 
    trace(f"cell_execute with eval={eval}")
+   if (allow_errors == True):
+      trace(f"cell_execute with allow_errors={allow_errors}")
 
    # execute if eval is active
    if eval == True:
@@ -553,6 +555,27 @@ def cell_execute(client, cell, index, execution_count, eval_default, store_histo
          if len(cell["metadata"]["tags"]) == 0:
             del cell["metadata"]["tags"]
 
+      # Check for display errors in output (respecting both global and cell settings)
+      cell_allows_errors = allow_errors if allow_errors is not None else client.allow_errors
+      if not cell_allows_errors:
+         trace("Cell does not allow errors: checking for uncaught errors")
+         for output in cell.outputs:
+            if output.get('output_type') == 'error':
+               trace("   Uncaught error found in output")
+               from nbclient.exceptions import CellExecutionError
+               error_name = output.get('ename', 'UnnamedError')
+               error_value = output.get('evalue', '')
+               traceback = output.get('traceback', [])
+               # Use same error raising mechanism as nbclient
+               raise CellExecutionError.from_cell_and_msg(
+                  cell,
+                  {
+                     'ename': 'UncaughtCellError:' + error_name,
+                     'evalue': error_value,
+                     'traceback': traceback
+                  }
+               )
+
    # return cell
    return cell
 

tests/docs/smoke-all/jupyter/error/.gitignore

@@ -0,0 +1 @@
+*.quarto_ipynb

tests/docs/smoke-all/jupyter/error/display-error-false.qmd

@@ -0,0 +1,45 @@
+---
+title: test
+format: html
+_quarto:
+  tests:
+    html:
+      shouldError: default
+      postRenderCleanup:
+        - '${input_stem}.quarto_ipynb'
+---
+
+With default setting, this document should error at rendering because of an exception at IPython.display level. 
+
+By default `nbconvert` does not throw exception for error thrown by IPython display, on purpose as document output is still valid as there are other representation. 
+
+```{python}
+# First cell - create an object with a buggy _repr_markdown_ method
+class BuggyDisplay:
+    def __init__(self):
+        self.data = "This works fine"
+    
+    def _repr_html_(self):
+        # HTML representation used for `format: html`
+        return "<b>HTML fallback:</b> " + self.data
+
+    def _repr_markdown_(self):
+        # This error happens during display, not execution
+        # even if the markdown reprensentation is not used
+        raise ValueError("Display phase error!")
+    
+    def __repr__(self):
+        # This ensures the object has a string representation
+        return self.data
+
+# Create the object
+buggy = BuggyDisplay()
+```
+
+```{python}
+buggy
+```
+
+```{python}
+print("Execution continued despite display error")
+```

tests/docs/smoke-all/jupyter/error/display-error-true-cell.qmd

@@ -0,0 +1,46 @@
+---
+title: test
+format: html
+_quarto:
+  tests:
+    html:
+      ensureHtmlElementContents: 
+        selectors: ['div.cell-output-error']
+        matches: ['ValueError\: Display phase error for HTML']
+        noMatches: []
+---
+
+With `error: true` in cell, this document should not error at rendering and Exception at IPython.display level should be shown in output. 
+
+By default `nbconvert` does not throw exception for error thrown by IPython display, on purpose as document output is still valid as there are other representation. 
+
+```{python}
+# First cell - create an object with a buggy _repr_html_ method
+class BuggyDisplay:
+    def __init__(self):
+        self.data = "This works fine"
+    
+    def _repr_html_(self):
+        # This error happens during display, not execution
+        raise ValueError("Display phase error for HTML!")
+
+    def _repr_markdown_(self):
+        # Markdown representation as fallback when HTML fails
+        return "**Markdown fallback:** " + self.data
+
+    def __repr__(self):
+        # This ensures the object has a string representation
+        return self.data
+
+# Create the object
+buggy = BuggyDisplay()
+```
+
+```{python}
+#| error: true
+buggy
+```
+
+```{python}
+print("Execution continued despite display error")
+```

tests/docs/smoke-all/jupyter/error/display-error-true-global-cell-false.qmd

@@ -0,0 +1,47 @@
+---
+title: test
+format: html
+execute:
+  error: true
+_quarto:
+  tests:
+    html:
+      shouldError: default
+      postRenderCleanup:
+        - '${input_stem}.quarto_ipynb'
+---
+
+With `error: true`, and `error: false` at cell level, this document should error at rendering.
+
+By default `nbconvert` does not throw exception for error thrown by IPython display, on purpose as document output is still valid as there are other representation. 
+
+```{python}
+class BuggyDisplay:
+    def __init__(self):
+        self.data = "This works fine"
+    
+    def _repr_html_(self):
+        # HTML representation used for `format: html`
+        return "<b>HTML fallback:</b> " + self.data
+
+    def _repr_markdown_(self):
+        # This error happens during display, not execution
+        # even if the markdown reprensentation is not used
+        raise ValueError("Display phase error for Markdown!")
+    
+    def __repr__(self):
+        # This ensures the object has a string representation
+        return self.data
+
+# Create the object
+buggy = BuggyDisplay()
+```
+
+```{python}
+#| error: false
+buggy
+```
+
+```{python}
+print("Execution continued despite display error")
+```

tests/docs/smoke-all/jupyter/error/display-error-true-global.qmd

@@ -0,0 +1,47 @@
+---
+title: test
+format: html
+execute:
+  error: true
+_quarto:
+  tests:
+    html:
+      ensureHtmlElementContents: 
+        selectors: ['div.cell-output-error']
+        matches: ['ValueError\: Display phase error for Markdown']
+---
+
+With `error: true` this document should not error at rendering and Exception at IPython.display level should be shown in output. 
+
+By default `nbconvert` does not throw exception for error thrown by IPython display, on purpose as document output is still valid as there are other representation. 
+
+```{python}
+# First cell - create an object with a buggy _repr_markdown_ method
+class BuggyDisplay:
+    def __init__(self):
+        self.data = "This works fine"
+    
+    def _repr_html_(self):
+        # HTML representation used for `format: html`
+        return "<b>HTML fallback:</b> " + self.data
+
+    def _repr_markdown_(self):
+        # This error happens during display, not execution
+        # even if the markdown reprensentation is not used
+        raise ValueError("Display phase error for Markdown!")
+    
+    def __repr__(self):
+        # This ensures the object has a string representation
+        return self.data
+
+# Create the object
+buggy = BuggyDisplay()
+```
+
+```{python}
+buggy
+```
+
+```{python}
+print("Execution continued despite display error")
+```