Skip to content

Commit

Permalink
‼️ Show errors in embedded notebooks (#181)
Browse files Browse the repository at this point in the history
Co-authored-by: Rowan Cockett <[email protected]>
  • Loading branch information
stevejpurves and rowanc1 authored Jul 17, 2023
1 parent dd2acd3 commit 88aa318
Show file tree
Hide file tree
Showing 8 changed files with 364 additions and 40 deletions.
6 changes: 6 additions & 0 deletions .changeset/six-kiwis-deliver.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@myst-theme/jupyter': patch
'@myst-theme/site': patch
---

Add Jupyter error tray for notebook execution errors
22 changes: 16 additions & 6 deletions packages/jupyter/src/ErrorTray.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react';
import type { PassiveCellRenderer } from 'thebe-core';
import type { IThebeNotebookError } from 'thebe-react';
import { useThebeLoader } from 'thebe-react';
import { useBusyErrors } from './execute/busy';

function ErrorDecoration({ children, idx }: React.PropsWithChildren<{ idx?: number }>) {
return (
Expand Down Expand Up @@ -38,7 +39,7 @@ function ErrorTrayMessage({ errors }: { errors: IThebeNotebookError[] }) {
return (
<div>
{errors.map((error, idx) => (
<div className="min-w-[400px]">
<div className="not-prose min-w-[400px]">
<ErrorDecoration idx={error.index}>
<div className="z-100" key={error.id} ref={refs[idx]}></div>
</ErrorDecoration>
Expand All @@ -48,13 +49,22 @@ function ErrorTrayMessage({ errors }: { errors: IThebeNotebookError[] }) {
);
}

export function ErrorTray({ errors }: { errors: IThebeNotebookError[] }) {
export function ErrorTray({ pageSlug, index }: { pageSlug: string; index?: string }) {
const { items } = useBusyErrors(pageSlug);
if (!items || items.length === 0) return null;
if (index && index) return null;
return (
<div className="relative px-4 pt-3 mt-8 text-sm text-red-600 border border-red-400 rounded border-1">
<div>
<span className="font-bold">Error</span> - a page refresh may resolve this.
</div>
<ErrorTrayMessage errors={errors} />
{items.map(({ notebookSlug, errors }) => {
return (
<div>
<div>
<span className="font-bold">Error</span> in notebook <span>"{notebookSlug}"</span>
</div>
<ErrorTrayMessage errors={errors} />
</div>
);
})}
</div>
);
}
149 changes: 145 additions & 4 deletions packages/jupyter/src/execute/busy.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useCallback, useReducer } from 'react';
import type { IThebeNotebookError } from 'thebe-react';

export type BusyKind = 'execute' | 'reset';

Expand All @@ -17,6 +18,11 @@ export interface BusyScopeState {
};
};
};
error: {
[pageSlug: string]: {
[notebookSlug: string]: IThebeNotebookError[];
};
};
}

export interface BusyScopeContext {
Expand Down Expand Up @@ -71,10 +77,33 @@ export interface NotebookPayload {
kind: BusyKind;
}

function isErrorPayload(payload: unknown): payload is ErrorPayload {
return (
(typeof (payload as SlugPayload).pageSlug === 'string' &&
typeof (payload as SlugPayload).notebookSlug === 'string' &&
(payload as ErrorPayload).errors === undefined) ||
((Array.isArray((payload as ErrorPayload).errors) &&
(payload as ErrorPayload).errors?.every((error) => typeof error === 'object')) ??
false)
);
}

export interface ErrorPayload {
pageSlug: string;
notebookSlug: string;
errors?: IThebeNotebookError[];
}

// TODO action typing is not giving build time type errors :(
type BusyScopeAction = {
type: 'SET_CELL_BUSY' | 'CLEAR_CELL_BUSY' | 'SET_NOTEBOOK_BUSY' | 'CLEAR_NOTEBOOK_BUSY';
payload: SlugPayload | CellPayload | NotebookPayload;
type:
| 'SET_CELL_BUSY'
| 'CLEAR_CELL_BUSY'
| 'SET_NOTEBOOK_BUSY'
| 'CLEAR_NOTEBOOK_BUSY'
| 'SET_ERROR'
| 'CLEAR_ERROR';
payload: SlugPayload | CellPayload | NotebookPayload | ErrorPayload;
};

export function reducer(state: BusyScopeState, action: BusyScopeAction): BusyScopeState {
Expand Down Expand Up @@ -206,12 +235,70 @@ export function reducer(state: BusyScopeState, action: BusyScopeAction): BusySco
},
};
}
case 'SET_ERROR': {
if (!isErrorPayload(action.payload)) {
console.error('SET_ERROR payload must be an error payload', action.payload);
return state;
}

const { pageSlug, notebookSlug, errors } = action.payload;

if (!errors) {
console.error('SET_ERROR payload must have errors', action.payload);
return state;
}
if (state.error[pageSlug]) return state;
if (state.error[pageSlug]?.[notebookSlug]) return state;

return {
...state,
error: {
...state.error,
[pageSlug]: {
...state.error[pageSlug],
[notebookSlug]: errors,
},
},
};
}
case 'CLEAR_ERROR': {
if (!isErrorPayload(action.payload)) {
console.error('CLEAR_ERROR payload must be a error payload', action.payload);
return state;
}

const { pageSlug, notebookSlug } = action.payload;

if (!state.error[pageSlug]) return state;
if (!state.error[pageSlug]?.[notebookSlug]) return state;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { [notebookSlug]: notebookErrors, ...otherNotebooks } = state.error[pageSlug];
if (Object.keys(otherNotebooks).length > 0) {
return {
...state,
error: {
...state.error,
[pageSlug]: {
...otherNotebooks,
},
},
};
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { [pageSlug]: renderErrors, ...otherRenders } = state.error;
return {
...state,
error: otherRenders,
};
}
}
return state;
}

export function BusyScopeProvider({ children }: React.PropsWithChildren) {
const [state, dispatch] = useReducer(reducer, { execute: {}, reset: {} });
const [state, dispatch] = useReducer(reducer, { execute: {}, reset: {}, error: {} });

const memo = React.useMemo(() => ({ state, dispatch }), [state]);

Expand Down Expand Up @@ -269,7 +356,61 @@ export function useBusyScope() {
[dispatch],
);

return { cell, notebook, page, setCell, clearCell, setNotebook, clearNotebook };
const setError = useCallback(
(pageSlug: string, notebookSlug: string, errors: IThebeNotebookError[]) =>
dispatch({ type: 'SET_ERROR', payload: { pageSlug, notebookSlug, errors } }),
[dispatch],
);

const clearError = useCallback(
(pageSlug: string, notebookSlug: string) =>
dispatch({ type: 'CLEAR_ERROR', payload: { pageSlug, notebookSlug } }),
[dispatch],
);

return {
cell,
notebook,
page,
setCell,
clearCell,
setNotebook,
clearNotebook,
setError,
clearError,
};
}

interface ErrorItem {
pageSlug: string;
notebookSlug: string;
errors: IThebeNotebookError[];
}

export function useBusyErrors(pageSlug: string) {
const context = React.useContext(BusyScopeContext);
if (context === undefined) {
throw new Error('useBusyScope must be used within a BusyScopeProvider');
}

const { state, dispatch } = context;

const clearErrors = () => {
Object.keys(state.error[pageSlug]).forEach((notebookSlug) => {
dispatch({ type: 'CLEAR_ERROR', payload: { pageSlug, notebookSlug } });
});
};

let items: ErrorItem[] | undefined;
if (Object.keys(state.error).length > 0 && state.error[pageSlug]) {
items = Object.entries(state.error[pageSlug]).map(([notebookSlug, errors]) => ({
pageSlug,
notebookSlug,
errors,
}));
}

return { items, clearErrors };
}

export function selectCellIsBusy(
Expand Down
11 changes: 9 additions & 2 deletions packages/jupyter/src/execute/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function useExecutionScope({
const execute = (slug: string) => {
// set busy
Object.entries(state.pages[slug].scopes).forEach(([notebookSlug, { notebook }]) => {
busy.clearError(slug, notebookSlug);
busy.setNotebook(
slug,
notebookSlug,
Expand Down Expand Up @@ -88,6 +89,7 @@ export function useExecutionScope({
(pageSlug: string) => {
Object.entries(state.pages[pageSlug]?.scopes).forEach(
([notebookSlug, { notebook, session }]) => {
busy.clearError(pageSlug, notebookSlug);
busy.setNotebook(
pageSlug,
notebookSlug,
Expand Down Expand Up @@ -149,7 +151,7 @@ export function useNotebookExecution(id: IdOrKey, clearOutputsOnExecute = false)

const execute = () => {
const nb = selectNotebookForPage(state, pageSlug, notebookSlug);

busy.clearError(pageSlug, notebookSlug);
// set busy
busy.setNotebook(
pageSlug,
Expand All @@ -171,7 +173,11 @@ export function useNotebookExecution(id: IdOrKey, clearOutputsOnExecute = false)
// execute all cells on the notebooks
const execReturns = await nb.executeAll(true);
const errs = findErrors(execReturns);
if (errs != null) console.error('errors', errs); // TODO: handle errors
if (errs != null) {
console.error('an error occurred during notebook execution');
busy.setError(pageSlug, notebookSlug, errs);
busy.clearNotebook(pageSlug, notebookSlug, 'execute');
}
config?.events.off('status' as any, handler);
}, 100);
};
Expand All @@ -190,6 +196,7 @@ export function useNotebookExecution(id: IdOrKey, clearOutputsOnExecute = false)
*/
const reset = useCallback(() => {
const nb = selectNotebookForPage(state, pageSlug, notebookSlug);
busy.clearError(pageSlug, notebookSlug);
busy.setNotebook(
pageSlug,
notebookSlug,
Expand Down
2 changes: 1 addition & 1 deletion packages/jupyter/src/execute/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {
selectSessionsToStart,
} from './selectors';
import { MdastFetcher, NotebookBuilder, ServerMonitor, SessionStarter } from './leaf';
import { useCanCompute } from '..';
import type { Thebe } from 'myst-frontmatter';
import { useCanCompute } from '../providers';

export interface ExecuteScopeType {
canCompute: boolean;
Expand Down
Loading

0 comments on commit 88aa318

Please sign in to comment.