Clear fix for 301 - handling promise reject inside KernelExecutor (#307)

* Clear fix for 301 - handling promise reject inside KernelExecutor * Fixed bubbling exception if no handler exists for KernelExecutor --------- Co-authored-by: Sergey Kadnikov <[email protected]>
datalayer · Sep 10, 2024 · 603337e · 603337e
1 parent 64c873f
commit 603337e
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 15 deletions.
diff --git a/packages/react/src/components/output/OutputAdapter.ts b/packages/react/src/components/output/OutputAdapter.ts
@@ -105,15 +105,10 @@ export class OutputAdapter {
         this._outputArea,
         this._kernel,
         metadata,
-        notifyOnComplete
+        notifyOnComplete,
+        onCodeExecutionError
       );
-      if (onCodeExecutionError) {
-        await done.catch(onCodeExecutionError);
-      } else {
-        await done.catch(err => {
-          console.error(err);
-        });
-      }
+      await done;
     }
   }
 

diff --git a/packages/react/src/components/output/OutputExecutor.ts b/packages/react/src/components/output/OutputExecutor.ts
@@ -18,7 +18,8 @@ export async function execute(
   output: OutputArea,
   kernel: Kernel,
   metadata?: JSONObject,
-  notifyOnComplete?: boolean
+  notifyOnComplete?: boolean,
+  onCodeExecutionError?: (err: any) => void
 ): Promise<KernelMessage.IExecuteReplyMsg | undefined> {
   // Override the default for `stop_on_error`.
   let stopOnError = true;
@@ -32,7 +33,8 @@ export async function execute(
   const kernelExecutor = kernel.execute(code, {
     model: output.model,
     stopOnError,
-    notifyOnComplete: notifyOnComplete,
+    notifyOnComplete,
+    onCodeExecutionError,
   });
   const future = kernelExecutor!.future;
   // TODO fix in upstream jupyterlab if possible...

diff --git a/packages/react/src/jupyter/kernel/Kernel.ts b/packages/react/src/jupyter/kernel/Kernel.ts
@@ -194,7 +194,8 @@ export class Kernel {
       stopOnError,
       storeHistory,
       allowStdin,
-      notifyOnComplete = false
+      notifyOnComplete = false,
+      onCodeExecutionError
     }: {
       model?: IOutputAreaModel;
       iopubMessageHooks?: IOPubMessageHook[];
@@ -203,14 +204,16 @@ export class Kernel {
       stopOnError?: boolean;
       storeHistory?: boolean;
       allowStdin?: boolean;
-      notifyOnComplete? : boolean
+      notifyOnComplete?: boolean
+      onCodeExecutionError?: (err:any) => void;
     } = {}
   ): KernelExecutor | undefined {
     if (this._kernelConnection) {
       const kernelExecutor = new KernelExecutor({
         connection: this._kernelConnection,
         model,
         notifyOnComplete,
+        onCodeExecutionError
       });
       kernelExecutor.execute(code, {
         iopubMessageHooks,

diff --git a/packages/react/src/jupyter/kernel/KernelExecutor.ts b/packages/react/src/jupyter/kernel/KernelExecutor.ts
@@ -46,6 +46,12 @@ export type IKernelExecutorOptions = {
    * must only be made when execution complete
    */
   notifyOnComplete? : boolean;
+  /**
+   * Handler for code execution error
+   * @param err Error data
+   * @returns 
+   */
+  onCodeExecutionError?: (err: any) => void;
 }
 
 export class KernelExecutor {
@@ -60,14 +66,16 @@ export class KernelExecutor {
   private _future?: JupyterKernel.IFuture<KernelMessage.IExecuteRequestMsg, KernelMessage.IExecuteReplyMsg>;
   private _shellMessageHooks = new Array<ShellMessageHook>();
   private _notifyOnComplete : boolean = false;
+  private _onCodeExecutionError?: (err: any) => void;
 
-  public constructor({ connection, model, notifyOnComplete }: IKernelExecutorOptions) {
+  public constructor({ connection, model, notifyOnComplete, onCodeExecutionError }: IKernelExecutorOptions) {
     this._executed = new PromiseDelegate<IOutputAreaModel>();
     this._kernelConnection = connection;
     this._model = model ?? new OutputAreaModel();
     this._outputs = [];
     this._kernelState = kernelsStore.getState();
     this._notifyOnComplete = !!notifyOnComplete;
+    this._onCodeExecutionError = onCodeExecutionError;
   }
 
   /**
@@ -163,7 +171,17 @@ export class KernelExecutor {
   get done(): Promise<void> {
     return this._executed.promise.then(() => {
       return;
-    });
+    })
+    .catch(
+      (err: any) => {
+        if (this._onCodeExecutionError) {
+          this._onCodeExecutionError(err);
+        } 
+        else {
+          throw err;
+        }
+      }
+    );
   }
 
   /**
@@ -172,7 +190,18 @@ export class KernelExecutor {
   get result(): Promise<string> {
     return this._executed.promise.then(model => {
       return outputsAsString(model.toJSON());
-    });
+    })
+    .catch(
+      (err: any) => {
+        if (this._onCodeExecutionError) {
+          this._onCodeExecutionError(err);
+          return "";
+        } 
+        else {
+          throw err;
+        }
+      }
+    );
   }
 
   /**