🛳 exposing any errors during execution in execute return object (#601)

jupyter-book · Feb 9, 2023 · 9910643 · 9910643
1 parent 3d03236
commit 9910643
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 36 deletions.
diff --git a/packages/core/src/cell.ts b/packages/core/src/cell.ts
@@ -130,7 +130,7 @@ class ThebeCell extends PassiveCellRenderer implements IThebeCell {
       console.debug(`thebe:renderer:execute ${this.id}`);
       if (!this.isBusy) this.setAsBusy();
 
-      const useShadow = true;
+      const useShadow = true; // TODO expose as option
       if (useShadow) {
         // Use a shadow output area for the execute request
         const model = new OutputAreaModel({ trusted: true });
@@ -151,7 +151,7 @@ class ThebeCell extends PassiveCellRenderer implements IThebeCell {
         await this.area.future.done;
       }
 
-      let hasExecuteErrors = false;
+      let executeErrors: IError[] | undefined;
       for (let i = 0; i < this.model.length; i++) {
         const out = this.model.get(i);
         if (out.type === 'error') {
@@ -162,7 +162,8 @@ class ThebeCell extends PassiveCellRenderer implements IThebeCell {
               message: errorToMessage(json),
             });
           } else {
-            hasExecuteErrors = true;
+            if (!executeErrors) executeErrors = [json];
+            else executeErrors?.push(json);
             this.events.triggerError({
               status: ErrorStatusEvent.executeError,
               message: errorToMessage(json),
@@ -176,7 +177,7 @@ class ThebeCell extends PassiveCellRenderer implements IThebeCell {
         id: this.id,
         height: this.area.node.offsetHeight,
         width: this.area.node.offsetWidth,
-        error: hasExecuteErrors,
+        error: executeErrors,
       };
     } catch (err: any) {
       console.error('thebe:renderer:execute Error:', err);

diff --git a/packages/core/src/cell_noexec.ts b/packages/core/src/cell_noexec.ts
@@ -110,6 +110,6 @@ export default class NonExecutableCell implements IThebeCell {
 
   async execute(source?: string): Promise<IThebeCellExecuteReturn | null> {
     // could potentially allow for markdown rendering here
-    return { id: this.id, height: 0, width: 0, error: false };
+    return { id: this.id, height: 0, width: 0 };
   }
 }
diff --git a/packages/core/src/notebook.ts b/packages/core/src/notebook.ts
@@ -10,12 +10,6 @@ import { EventEmitter } from './emitter';
 import type { ICodeCell, INotebookContent, INotebookMetadata } from '@jupyterlab/nbformat';
 import NonExecutableCell from './cell_noexec';
 
-export interface ExecuteReturn {
-  id: string;
-  height: number;
-  width: number;
-}
-
 export interface CodeBlock {
   id: string;
   source: string;
@@ -150,7 +144,7 @@ class ThebeNotebook {
     cellId: string,
     stopOnError = false,
     preprocessor?: (s: string) => string,
-  ): Promise<(ExecuteReturn | null)[]> {
+  ): Promise<(IThebeCellExecuteReturn | null)[]> {
     if (!this.cells) return [];
     this.events.triggerStatus({
       status: NotebookStatusEvent.executing,
@@ -177,7 +171,7 @@ class ThebeNotebook {
   async executeOnly(
     cellId: string,
     preprocessor?: (s: string) => string,
-  ): Promise<ExecuteReturn | null> {
+  ): Promise<IThebeCellExecuteReturn | null> {
     if (!this.cells) return null;
     this.events.triggerStatus({
       status: NotebookStatusEvent.executing,
@@ -196,7 +190,7 @@ class ThebeNotebook {
     cellIds: string[],
     stopOnError = false,
     preprocessor?: (s: string) => string,
-  ): Promise<(ExecuteReturn | null)[]> {
+  ): Promise<(IThebeCellExecuteReturn | null)[]> {
     if (!this.cells) return [];
     this.events.triggerStatus({
       status: NotebookStatusEvent.executing,
@@ -238,7 +232,7 @@ class ThebeNotebook {
   async executeAll(
     stopOnError = false,
     preprocessor?: (s: string) => string,
-  ): Promise<(ExecuteReturn | null)[]> {
+  ): Promise<(IThebeCellExecuteReturn | null)[]> {
     if (!this.cells) return [];
 
     this.events.triggerStatus({

diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
@@ -1,6 +1,6 @@
 import type { KernelSpecAPI, ServerConnection, Session } from '@jupyterlab/services';
 import type ThebeSession from './session';
-import type { IOutput } from '@jupyterlab/nbformat';
+import type { IOutput, IError } from '@jupyterlab/nbformat';
 import type { IRenderMimeRegistry } from '@jupyterlab/rendermime';
 import type ThebeServer from './server';
 import type { ServerStatusEvent } from './events';
@@ -104,7 +104,7 @@ export interface IThebeCellExecuteReturn {
   id: string;
   height: number;
   width: number;
-  error: boolean;
+  error?: IError[];
 }
 
 export interface ServerRuntime {

diff --git a/packages/react/src/hooks/notebook.ts b/packages/react/src/hooks/notebook.ts
@@ -1,23 +1,40 @@
 import { createRef, useEffect, useState } from 'react';
-import type { ThebeNotebook, ThebeSession, IThebeCell } from 'thebe-core';
+import type { ThebeNotebook, ThebeSession, IThebeCell, IThebeCellExecuteReturn } from 'thebe-core';
 import { useThebeConfig } from '../ThebeServerProvider';
 import { useThebeCore } from '../ThebeCoreProvider';
 import type { INotebookContent } from '@jupyterlab/nbformat';
 import { useThebeSession } from '../ThebeSessionProvider';
 
-interface ExecuteOptions {
+export interface NotebookExecuteOptions {
+  stopOnError?: boolean;
   before?: () => void;
   after?: () => void;
   preprocessor?: (s: string) => string;
 }
 
-function useNotebookBase(opts = { refsForWidgetsOnly: true }) {
+export type IThebeNotebookError = IThebeCellExecuteReturn & { index: number };
+
+function findErrors(execReturns: (IThebeCellExecuteReturn | null)[]) {
+  return execReturns.reduce<IThebeNotebookError[] | null>(
+    (acc, retval: IThebeCellExecuteReturn | null, index) => {
+      if (retval?.error) {
+        if (acc == null) return [{ ...retval, index }];
+        else return [...acc, { ...retval, index }];
+      }
+      return acc;
+    },
+    null,
+  );
+}
+
+function useNotebookBase() {
   const { session } = useThebeSession();
   const [notebook, setNotebook] = useState<ThebeNotebook | undefined>();
   const [refs, setRefs] = useState<((node: HTMLDivElement) => void)[]>([]);
   const [sessionAttached, setSessionAttached] = useState(false);
   const [executing, setExecuting] = useState<boolean>(false);
   const [executed, setExecuted] = useState(false);
+  const [errors, setErrors] = useState<IThebeNotebookError[] | null>(null);
 
   /**
    * When the notebook and session is avaiable, attach to session
@@ -28,29 +45,40 @@ function useNotebookBase(opts = { refsForWidgetsOnly: true }) {
     setSessionAttached(true);
   }, [notebook, session]);
 
-  const executeAll = (options?: ExecuteOptions) => {
+  const executeAll = (options?: NotebookExecuteOptions) => {
     if (!notebook) throw new Error('executeAll called before notebook available');
     options?.before?.();
     setExecuting(true);
-    return notebook.executeAll(true, options?.preprocessor).then((exec_return) => {
-      options?.after?.();
-      setExecuted(true);
-      setExecuting(false);
-      return exec_return;
-    });
+    return notebook
+      .executeAll(options?.stopOnError ?? true, options?.preprocessor)
+      .then((execReturns) => {
+        options?.after?.();
+        const errs = findErrors(execReturns);
+        if (errs != null) setErrors(errs);
+        setExecuted(true);
+        setExecuting(false);
+        return execReturns;
+      });
   };
 
-  const executeSome = (predicate: (cell: IThebeCell) => boolean, options?: ExecuteOptions) => {
+  const executeSome = (
+    predicate: (cell: IThebeCell) => boolean,
+    options?: NotebookExecuteOptions,
+  ) => {
     if (!notebook) throw new Error('executeSome called before notebook available');
     options?.before?.();
     setExecuting(true);
     const filteredCells = notebook.cells.filter(predicate).map((c) => c.id);
-    return notebook.executeCells(filteredCells, true, options?.preprocessor).then((exec_return) => {
-      options?.after?.();
-      setExecuted(true);
-      setExecuting(false);
-      return exec_return;
-    });
+    return notebook
+      .executeCells(filteredCells, options?.stopOnError ?? true, options?.preprocessor)
+      .then((execReturns) => {
+        options?.after?.();
+        const errs = findErrors(execReturns);
+        if (errs != null) setErrors(errs);
+        setExecuted(true);
+        setExecuting(false);
+        return execReturns;
+      });
   };
 
   const clear = () => {
@@ -64,6 +92,7 @@ function useNotebookBase(opts = { refsForWidgetsOnly: true }) {
     attached: sessionAttached,
     executing,
     executed,
+    errors,
     notebook,
     setNotebook,
     refs,
@@ -95,6 +124,7 @@ export function useNotebook(
     attached,
     executing,
     executed,
+    errors,
     notebook,
     setNotebook,
     refs,
@@ -103,7 +133,7 @@ export function useNotebook(
     executeSome,
     clear,
     session,
-  } = useNotebookBase(opts);
+  } = useNotebookBase();
 
   /**
    * - set loading flag
@@ -141,6 +171,7 @@ export function useNotebook(
     attached,
     executing,
     executed,
+    errors,
     notebook,
     cellRefs: refs,
     cellIds: (opts.refsForWidgetsOnly ? notebook?.widgets ?? [] : notebook?.cells ?? []).map(
@@ -167,6 +198,7 @@ export function useNotebookFromSource(sourceCode: string[], opts = { refsForWidg
     attached,
     executing,
     executed,
+    errors,
     notebook,
     setNotebook,
     refs,
@@ -175,7 +207,7 @@ export function useNotebookFromSource(sourceCode: string[], opts = { refsForWidg
     executeSome,
     clear,
     session,
-  } = useNotebookBase(opts);
+  } = useNotebookBase();
 
   useEffect(() => {
     if (!core || !config) return;
@@ -203,6 +235,7 @@ export function useNotebookFromSource(sourceCode: string[], opts = { refsForWidg
     attached,
     executing,
     executed,
+    errors,
     notebook,
     cellRefs: refs,
     cellIds: (opts.refsForWidgetsOnly ? notebook?.widgets ?? [] : notebook?.cells ?? []).map(