Skip to content

improving execution error handling #473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/core/src/ExecutionMemoryFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Hook } from './types/Hook'
import { ItemValue } from './types/ItemValue'
import { LinkId } from './types/Link'
import { Node, NodeId } from './types/Node'
import { withNodeExecutionErrorHandling } from './utils/withNodeExecutionErrorHandling'

export class ExecutionMemoryFactory {
constructor(
Expand Down Expand Up @@ -64,7 +65,7 @@ export class ExecutionMemoryFactory {

// Initialize runner context
const context = new NodeRunnerContext(node.id);
context.status = computer.run({
context.status = withNodeExecutionErrorHandling(computer.run, node)({
input: inputDevice,
output: outputDevice,
params: this.makeParamsDevice(node, memory),
Expand Down
18 changes: 9 additions & 9 deletions packages/core/src/ItemWithParams/ItemWithParams.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { ItemValue } from '../types/ItemValue';
import { Param, EvaluatedParamValue } from '../Param';
import { EvaluatedParamValue, Param } from '../Param';
import { ParamEvaluator } from './ParamEvaluator';

export const isItemWithParams = (item: ItemWithParams | unknown): item is ItemWithParams => {
// This does not always catch all cases
if(item instanceof ItemWithParams) return true;
if (item instanceof ItemWithParams) return true;

if(
if (
item !== null
&& typeof item === 'object'
// @ts-ignore
Expand All @@ -16,10 +16,10 @@ export const isItemWithParams = (item: ItemWithParams | unknown): item is ItemWi
) return true;

return false;
}
};

export class ItemWithParams<ExpectedType extends ItemValue = ItemValue> {
type = 'ItemWithParams' as const
type = 'ItemWithParams' as const;
value: ExpectedType;
params: Record<string, EvaluatedParamValue>;

Expand All @@ -37,11 +37,11 @@ export class ItemWithParams<ExpectedType extends ItemValue = ItemValue> {
try {
const paramEvaluatorInstance = new ParamEvaluator();
return paramEvaluatorInstance.evaluate(value, param, globalParams);
} catch (error) {
console.error('error', error);
return param.input;
} catch (e) {
console.error('Failed while evaluating param', param, e);
throw e;
}
},
});
}
}
}
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,4 @@ export type { Param, ParamInput as ParamValue } from './Param'
export type { Port, AbstractPort } from './types/Port'
export type { ReportCallback } from './types/ReportCallback';
export type { ServiceProvider } from './types/ServiceProvider'
export { Table } from './computers'
export { Table } from './computers'
8 changes: 8 additions & 0 deletions packages/core/src/types/Errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Node } from './Node';

export class NodeExecutionError extends Error {
constructor(message: string, public node: Pick<Node, 'id' | 'label' | 'type'>) {
super(`Error in node ${node.label || node.id} (${node.type}): ${message}`);
this.name = 'NodeExecutionError';
}
}
90 changes: 90 additions & 0 deletions packages/core/src/utils/withNodeExecutionErrorHandling.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { describe, expect, it } from 'vitest';
import { withNodeExecutionErrorHandling } from './withNodeExecutionErrorHandling';
import { NodeExecutionError } from '../types/Errors';

describe('withNodeExecutionErrorHandling', () => {
const mockNode = {
id: 'test-node-1',
label: 'Test Node',
type: 'TestComputer',
};

it('should yield values normally when no error occurs', async () => {
const mockAsyncGenerator = async function* () {
yield 'value1';
yield 'value2';
return 'final';
};

const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
const generator = wrappedGenerator();

const result1 = await generator.next();
expect(result1.value).toBe('value1');
expect(result1.done).toBe(false);

const result2 = await generator.next();
expect(result2.value).toBe('value2');
expect(result2.done).toBe(false);

const result3 = await generator.next();
expect(result3.value).toBe('final');
expect(result3.done).toBe(true);
});

it('should wrap errors with NodeExecutionError', async () => {
const errorMessage = 'Something went wrong';
const mockAsyncGenerator = async function* () {
yield 'value1';
throw new Error(errorMessage);
};

const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
const generator = wrappedGenerator();

const result1 = await generator.next();
expect(result1.value).toBe('value1');

await expect(generator.next()).rejects.toThrow(NodeExecutionError);

try {
await generator.next();
} catch (error) {
expect(error).toBeInstanceOf(NodeExecutionError);
expect((error as NodeExecutionError).node).toEqual(mockNode);
expect((error as NodeExecutionError).message).toContain(errorMessage);
expect((error as NodeExecutionError).message).toContain(mockNode.label);
expect((error as NodeExecutionError).message).toContain(mockNode.type);
}
});

it('should handle non-Error objects thrown', async () => {
const mockAsyncGenerator = async function* () {
throw 'string error';
};

const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
const generator = wrappedGenerator();

await expect(generator.next()).rejects.toThrow(NodeExecutionError);

try {
await generator.next();
} catch (error) {
expect(error).toBeInstanceOf(NodeExecutionError);
expect((error as NodeExecutionError).message).toContain('string error');
}
});

it('should pass arguments correctly to the wrapped function', async () => {
const mockAsyncGenerator = async function* (arg1: string, arg2: number) {
yield `${arg1}-${arg2}`;
};

const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
const generator = wrappedGenerator('test', 42);

const result = await generator.next();
expect(result.value).toBe('test-42');
});
});
24 changes: 24 additions & 0 deletions packages/core/src/utils/withNodeExecutionErrorHandling.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { NodeExecutionError } from '../types/Errors';
import { Node } from '../types/Node';

/**
* High-order helper function that wraps an async generator function
* and rethrows any error thrown by the run function wrapped with NodeExecutionError
*
* @param runFn - The async generator function to wrap
* @param node - The node context for error reporting
* @returns A wrapped async generator that handles errors
*/
export function withNodeExecutionErrorHandling<TArgs extends any[], TYield, TReturn, TNext>(
runFn: (...args: TArgs) => AsyncGenerator<TYield, TReturn, TNext>,
node: Pick<Node, 'id' | 'label' | 'type'>,
) {
return async function* (...args: TArgs): AsyncGenerator<TYield, TReturn, TNext> {
try {
return yield* runFn(...args);
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error);
throw new NodeExecutionError(errorMessage, node);
}
};
}
2 changes: 1 addition & 1 deletion packages/ds-ext/src/app/VsCodeToast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function VsCodeToast({ postMessage }: {
`;
break;
case DataStoryEvents.RUN_ERROR:
info.message = `Diagram execution failed! Error was: ${event.payload.error}`;
info.message = `Diagram execution failed! Error was: ${event.payload.message}`;
info.status = 'error';
break;
case DataStoryEvents.SAVE_SUCCESS:
Expand Down