[Fix] Update Cell execution result and align client and server state

changelogs/current_changelog.md

@@ -110,6 +110,7 @@ Provide a bulleted list of bug fixes and a reference to the PR(s) containing the
 #### Breaking Changes
 
 Provide a bulleted list of breaking changes and a reference to the PR(s) containing those changes.
+- execute cell is now synchronized with kernel execute_reply returned status and will not stop following cells while the kernel still executes. ([PR5561](https://github.com/nteract/nteract/pull/5561))
 
 #### New Features
 
@@ -166,10 +167,12 @@ Provide a bulleted list of bug fixes and a reference to the PR(s) containing the
 #### Breaking Changes
 
 Provide a bulleted list of breaking changes and a reference to the PR(s) containing those changes.
+- executeRequest uses Jupyter default for stop_on_error = True. ([PR5561](https://github.com/nteract/nteract/pull/5561))
 
 #### New Features
 
 Provide a bulleted list of new features or improvements and a reference to the PR(s) containing these changes.
+- Add support for retrieving executionErrors including aborted requests ([PR5561](https://github.com/nteract/nteract/pull/5561))
 
 #### Bug Fixes
 

packages/actions/__tests__/actions-spec.ts

@@ -317,6 +317,23 @@ describe("updateCellStatus", () => {
   });
 });
 
+describe("updateCellExecutionResult", () => {
+  test("creates an UPDATE_CELL_EXECUTION_RESULT action", () => {
+    const contentRef = createContentRef();
+    expect(
+      actions.updateCellExecutionResult({ id: "1234", result: "test", contentRef })
+    ).toEqual({
+      type: actionTypes.UPDATE_CELL_EXECUTION_RESULT,
+      payload: {
+        id: "1234",
+        contentRef,
+        result: "test"
+      }
+    });
+  });
+});
+
+
 describe("moveCell", () => {
   test("creates a MOVE_CELL action", () => {
     const contentRef = createContentRef();

packages/actions/src/actionTypes/cell_state.ts

@@ -2,27 +2,30 @@
 import { CellType } from "@nteract/commutable";
 import { Action, HasCell, makeActionFunction, MaybeHasCell } from "../utils";
 
-export const TOGGLE_TAG_IN_CELL       = "CORE/TOGGLE_TAG_IN_CELL";
-export const CHANGE_CELL_TYPE         = "CHANGE_CELL_TYPE";
-export const UPDATE_CELL_STATUS       = "UPDATE_CELL_STATUS";
-export const MARK_CELL_AS_DELETING    = "MARK_CELL_AS_DELETING";
-export const UNMARK_CELL_AS_DELETING  = "UNMARK_CELL_AS_DELETING";
-export const SET_IN_CELL              = "SET_IN_CELL";
+export const TOGGLE_TAG_IN_CELL            = "CORE/TOGGLE_TAG_IN_CELL";
+export const CHANGE_CELL_TYPE              = "CHANGE_CELL_TYPE";
+export const UPDATE_CELL_STATUS            = "UPDATE_CELL_STATUS";
+export const MARK_CELL_AS_DELETING         = "MARK_CELL_AS_DELETING";
+export const UNMARK_CELL_AS_DELETING       = "UNMARK_CELL_AS_DELETING";
+export const SET_IN_CELL                   = "SET_IN_CELL";
+export const UPDATE_CELL_EXECUTION_RESULT  = "UPDATE_CELL_EXECUTION_RESULT";
 
-export type ChangeCellType            = Action<typeof CHANGE_CELL_TYPE,         MaybeHasCell & { to: CellType }>;
-export type UpdateCellStatus          = Action<typeof UPDATE_CELL_STATUS,       HasCell      & { status: string }>;
-export type ToggleTagInCell           = Action<typeof TOGGLE_TAG_IN_CELL,       HasCell      & { tag: string }>;
-export type MarkCellAsDeleting        = Action<typeof MARK_CELL_AS_DELETING,    HasCell>;
-export type UnmarkCellAsDeleting      = Action<typeof UNMARK_CELL_AS_DELETING,  HasCell>;
-export type SetInCell             <T> = Action<typeof SET_IN_CELL,              HasCell      & { path: string[]; value: T }>;
+export type ChangeCellType                 = Action<typeof CHANGE_CELL_TYPE,                  MaybeHasCell & { to: CellType }>;
+export type UpdateCellStatus               = Action<typeof UPDATE_CELL_STATUS,                HasCell      & { status: string }>;
+export type ToggleTagInCell                = Action<typeof TOGGLE_TAG_IN_CELL,                HasCell      & { tag: string }>;
+export type MarkCellAsDeleting             = Action<typeof MARK_CELL_AS_DELETING,             HasCell>;
+export type UnmarkCellAsDeleting           = Action<typeof UNMARK_CELL_AS_DELETING,           HasCell>;
+export type SetInCell             <T>      = Action<typeof SET_IN_CELL,                       HasCell      & { path: string[]; value: T }>;
+export type UpdateCellExecutionResult      = Action<typeof UPDATE_CELL_EXECUTION_RESULT,      HasCell      & { result: string; }>;
+
+export const changeCellType                = makeActionFunction<ChangeCellType>            (CHANGE_CELL_TYPE);
+export const updateCellStatus              = makeActionFunction<UpdateCellStatus>          (UPDATE_CELL_STATUS);
+export const toggleTagInCell               = makeActionFunction<ToggleTagInCell>           (TOGGLE_TAG_IN_CELL);
+export const markCellAsDeleting            = makeActionFunction<MarkCellAsDeleting>        (MARK_CELL_AS_DELETING);
+export const unmarkCellAsDeleting          = makeActionFunction<UnmarkCellAsDeleting>      (UNMARK_CELL_AS_DELETING);
+export const updateCellExecutionResult     = makeActionFunction<UpdateCellExecutionResult> (UPDATE_CELL_EXECUTION_RESULT);
 
-export const changeCellType           = makeActionFunction<ChangeCellType>      (CHANGE_CELL_TYPE);
-export const updateCellStatus         = makeActionFunction<UpdateCellStatus>    (UPDATE_CELL_STATUS);
-export const toggleTagInCell          = makeActionFunction<ToggleTagInCell>     (TOGGLE_TAG_IN_CELL);
-export const markCellAsDeleting       = makeActionFunction<MarkCellAsDeleting>  (MARK_CELL_AS_DELETING);
-export const unmarkCellAsDeleting     = makeActionFunction<UnmarkCellAsDeleting>(UNMARK_CELL_AS_DELETING);
-
-export const setInCell                = <T>(payload: SetInCell<T>["payload"])   => makeActionFunction<SetInCell<T>>(SET_IN_CELL)(payload);
-export const toggleParameterCell      = (payload: HasCell)                      => toggleTagInCell({...payload, tag: "parameters"}); // Tag comes via Papermill
-export const updateCellSource         = (payload: HasCell & { value: string })  => setInCell({...payload, path: ["source"]});
-export const updateCellExecutionCount = (payload: HasCell & { value: number })  => setInCell({...payload, path: ["execution_count"]});
+export const setInCell                     = <T>(payload: SetInCell<T>["payload"])   => makeActionFunction<SetInCell<T>>(SET_IN_CELL)(payload);
+export const toggleParameterCell           = (payload: HasCell)                      => toggleTagInCell({...payload, tag: "parameters"}); // Tag comes via Papermill
+export const updateCellSource              = (payload: HasCell & { value: string })  => setInCell({...payload, path: ["source"]});
+export const updateCellExecutionCount      = (payload: HasCell & { value: number })  => setInCell({...payload, path: ["execution_count"]});

packages/actions/src/actionTypes/kernel_execution.ts

@@ -18,7 +18,7 @@ export type ExecuteCell               = Action     <typeof EXECUTE_CELL,
 export type ExecuteAllCells           = Action     <typeof EXECUTE_ALL_CELLS,       HasContent>;
 export type ExecuteAllCellsBelow      = Action     <typeof EXECUTE_ALL_CELLS_BELOW, HasContent>;
 export type ExecuteFocusedCell        = Action     <typeof EXECUTE_FOCUSED_CELL,    HasContent>;
-export type ExecuteCanceled           = Action     <typeof EXECUTE_CANCELED,        HasCell & {code?: string}>;
+export type ExecuteCanceled           = Action     <typeof EXECUTE_CANCELED,        HasCell & {code?: string; error?: any}>;
 export type ExecuteFailed             = ErrorAction<typeof EXECUTE_FAILED,          MaybeHasContent & { id?: CellId }>;
 export type SetExecutionStateAction   = Action     <typeof SET_EXECUTION_STATE,     HasKernel &  { kernelStatus: string }>;
 export type EnqueueAction             = Action     <typeof ENQUEUE_ACTION,          HasCell>;

packages/epics/__tests__/execute/execute.spec.ts

@@ -60,7 +60,11 @@ describe("executeCellStream", () => {
           msg_type: "execute_reply"
         },
         content: {
-          execution_count: 0
+          execution_count: 1, 
+          status: "error",
+          ename: "TestException", 
+          evalue: "testEvalue", 
+          traceback: ["1", "2"]
         }
       }
     ];
@@ -124,6 +128,62 @@ describe("executeCellStream", () => {
       )
     );
 
+    expect(emittedActions).toContainEqual(
+      expect.objectContaining(
+        actions.updateCellStatus({
+          id: "0",
+          contentRef: "fakeContentRef",
+          status: "idle"
+        })
+      )
+    );
+
+    expect(emittedActions).toContainEqual(
+      expect.objectContaining(
+        actions.updateCellStatus({
+          id: "0",
+          contentRef: "fakeContentRef",
+          status: "busy"
+        })
+      )
+    );
+
+    expect(emittedActions).toContainEqual(
+      expect.objectContaining(
+        actions.updateCellExecutionResult({
+          id: "0",
+          contentRef: "fakeContentRef",
+          result: "error"
+        })
+      )
+    );
+
+    expect(emittedActions).toContainEqual(
+      expect.objectContaining(
+        actions.updateCellExecutionCount({
+          id: "0",
+          contentRef: "fakeContentRef",
+          value: 1
+        })
+      )
+    );
+
+    expect(emittedActions).toContainEqual(
+      expect.objectContaining(
+        actions.executeCanceled({
+          id: "0",
+          contentRef: "fakeContentRef",
+          code: "EXEC_CELL_RUNTIME_ERROR",
+          error: {
+            execution_count: 1, 
+            status: "error",
+            ename: "TestException", 
+            evalue: "testEvalue", 
+            traceback: ["1", "2"]
+          }
+        })
+      )
+    );
     done();
   });
 });

packages/epics/src/execute/execute.ts

@@ -4,6 +4,8 @@ import {
   executeRequest,
   ExecuteRequest,
   executionCounts,
+  executionStatuses,
+  executionErrors, 
   inputRequests,
   JupyterMessage,
   kernelStatuses,
@@ -35,11 +37,11 @@ import {
   ContentRef,
   InputRequestMessage,
   KernelStatus,
-  PayloadMessage,
+  PayloadMessage, 
   errors
 } from "@nteract/types";
 
-const EXECUTE_CANCEL_ALL = "all";
+import { ExecuteReplyError } from "@nteract/messaging";
 
 /**
  * Observe all the reactions to running code for cell with id.
@@ -131,6 +133,14 @@ export function executeCellStream(
       )
     ),
 
+    // Update the cell execution result from execute_reply.content.status
+    cellMessages.pipe(
+      executionStatuses() as any,
+      map((result: string) =>
+        actions.updateCellExecutionResult({ id, result, contentRef })
+      )
+    ),
+
     // Update the input numbering: `[ ]`
     cellMessages.pipe(
       executionCounts() as any,
@@ -148,8 +158,10 @@ export function executeCellStream(
     ),
 
     cellMessages.pipe(
-      ofMessageType("error"),
-      map(() => actions.executeCanceled({ contentRef, id: EXECUTE_CANCEL_ALL }))
+      executionErrors() as any,
+      map((error: ExecuteReplyError) => 
+        actions.executeCanceled({ contentRef, id, code: errors.EXEC_CELL_RUNTIME_ERROR, error })
+      )
     ),
 
     // clear_output display message
@@ -226,9 +238,7 @@ export function createExecuteCellStream(
         action$.pipe(
           ofType(actions.EXECUTE_CANCELED, actions.DELETE_CELL),
           filter(
-            (action: ExecuteStreamActions) =>
-              (action as PerCellStopStopExecutionActions).payload.id === id ||
-              (action as PerCellStopStopExecutionActions).payload.id === EXECUTE_CANCEL_ALL
+            (action: ExecuteStreamActions) => (action as PerCellStopStopExecutionActions).payload.id === id 
           )
         ),
         action$.pipe(

packages/messaging/__tests__/messaging-spec.ts

@@ -15,10 +15,13 @@ import {
   kernelStatuses,
   ofMessageType,
   outputs,
-  payloads
+  payloads,
+  executionStatuses, 
+  executionErrors
 } from "../src";
 import {
   displayData,
+  error,
   executeInput,
   executeReply,
   message,
@@ -366,6 +369,63 @@ describe("executionCounts", () => {
   });
 });
 
+describe("executionStatuses", () => {
+  it("extracts all execution status from a session", () => {
+    return of(
+      status(KernelStatus.Starting),
+      status(KernelStatus.Idle),
+      status(KernelStatus.Busy),
+      executeReply({
+        status: "ok",
+        execution_count: 0
+      }),
+      displayData({ data: { "text/plain": "woo" } }),
+      displayData({ data: { "text/plain": "hoo" } }),
+      executeReply({
+        status: "aborted",
+        execution_count: 1
+      }),
+      status(KernelStatus.Idle)
+    )
+      .pipe(executionStatuses(), toArray())
+      .toPromise()
+      .then(arr => {
+        expect(arr).toEqual(["ok", "aborted"]);
+      });
+  });
+});
+
+describe("error", () => {
+  it("extracts all error from a session", () => {
+    const errorContent = {
+      ename: "TestException", 
+      evalue: "testEvalue", 
+      traceback: ["1", "2"]
+    };
+    return of(
+      status(KernelStatus.Starting),
+      status(KernelStatus.Idle),
+      status(KernelStatus.Busy),
+      executeReply({
+        status: "error",
+        execution_count: 0,
+        ...errorContent
+      }),
+      error(errorContent),
+      status(KernelStatus.Idle)
+    )
+      .pipe(executionErrors(), toArray())
+      .toPromise()
+      .then(arr => {
+        expect(arr).toEqual([{
+          status: "error",
+          execution_count: 0,
+          ...errorContent
+        }]);
+      });
+  });
+});
+
 describe("kernelStatuses", () => {
   it("extracts all the execution states from status messages", () => {
     return of(

packages/messaging/src/index.ts

@@ -245,6 +245,26 @@ export const executionCounts = () => (source: Observable<JupyterMessage>) =>
     map(entry => entry.content.execution_count)
   );
 
+/**
+ * Get all the execution status from the observable of the jupyter messages
+ * One of: 'ok' OR 'error' OR 'aborted' from the messaging protocol: https://jupyter-client.readthedocs.io/en/stable/messaging.html#execution-results
+ */
+export const executionStatuses = () => (source: Observable<JupyterMessage>) =>
+source.pipe(
+  ofMessageType("execute_reply"),
+  map(entry => entry.content.status)
+);
+
+/**
+ * Get all execution errors from the observable of the jupyter messages
+ */
+export const executionErrors = () => (source: Observable<JupyterMessage>) =>
+source.pipe(
+  ofMessageType("execute_reply"),
+  filter((entry) => entry.content.status !== "ok"),
+  map(entry => entry.content)
+);
+
 /**
  * Get all statuses of all running kernels.
  */

packages/messaging/src/messages.ts

@@ -125,6 +125,7 @@ function createHeader<MT extends MessageType>(
 /**
  * An execute request creator
  *
+ * ref: https://jupyter-client.readthedocs.io/en/stable/messaging.html#execute
  * > executeRequest('print("hey")', { 'silent': true })
  * { header:
  *    { msg_id: 'f344cc6b-4308-4405-a8e8-a166b0345579',
@@ -141,7 +142,7 @@ function createHeader<MT extends MessageType>(
  *      store_history: true,
  *      user_expressions: {},
  *      allow_stdin: true,
- *      stop_on_error: false } }
+ *      stop_on_error: true } }
  *
  * @param code The code to execute
  * @param options The options for the execute request
@@ -170,7 +171,7 @@ export function executeRequest(
       store_history: true,
       user_expressions: {},
       allow_stdin: true,
-      stop_on_error: false,
+      stop_on_error: true,
       ...options
     },
     channel,
@@ -270,11 +271,14 @@ export function executeResult(content: {
  *
  * http://jupyter-client.readthedocs.io/en/stable/messaging.html#execution-errors
  */
-export function error(content: {
+
+export interface ExecuteError {
   ename?: string;
   evalue?: string;
   traceback?: string[];
-}) {
+}
+
+export function error(content: ExecuteError) {
   return message(
     {
       msg_type: "error"