From 280c537e78ee4d119df86b619eec72368ef1df6d Mon Sep 17 00:00:00 2001 From: roy Date: Sun, 27 Jul 2025 16:01:16 +0800 Subject: [PATCH 1/3] fix: Correct error message source in VS Code toast --- packages/ds-ext/src/app/VsCodeToast.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ds-ext/src/app/VsCodeToast.tsx b/packages/ds-ext/src/app/VsCodeToast.tsx index 1737a67a..8f0f1c58 100644 --- a/packages/ds-ext/src/app/VsCodeToast.tsx +++ b/packages/ds-ext/src/app/VsCodeToast.tsx @@ -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: From 9034779d602e008ee088b5c50d674be69b9f9f76 Mon Sep 17 00:00:00 2001 From: roy Date: Sun, 27 Jul 2025 16:48:37 +0800 Subject: [PATCH 2/3] feat: Add structured error handling for node execution --- packages/core/src/ExecutionMemoryFactory.ts | 3 +- .../core/src/ItemWithParams/ItemWithParams.ts | 18 ++-- packages/core/src/index.ts | 2 +- packages/core/src/types/Errors.ts | 8 ++ .../withNodeExecutionErrorHandling.test.ts | 90 +++++++++++++++++++ .../utils/withNodeExecutionErrorHandling.ts | 24 +++++ 6 files changed, 134 insertions(+), 11 deletions(-) create mode 100644 packages/core/src/types/Errors.ts create mode 100644 packages/core/src/utils/withNodeExecutionErrorHandling.test.ts create mode 100644 packages/core/src/utils/withNodeExecutionErrorHandling.ts diff --git a/packages/core/src/ExecutionMemoryFactory.ts b/packages/core/src/ExecutionMemoryFactory.ts index ef897091..0275f3dd 100644 --- a/packages/core/src/ExecutionMemoryFactory.ts +++ b/packages/core/src/ExecutionMemoryFactory.ts @@ -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( @@ -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.bind(computer), node)({ input: inputDevice, output: outputDevice, params: this.makeParamsDevice(node, memory), diff --git a/packages/core/src/ItemWithParams/ItemWithParams.ts b/packages/core/src/ItemWithParams/ItemWithParams.ts index ba9d9ac1..d2d4f778 100644 --- a/packages/core/src/ItemWithParams/ItemWithParams.ts +++ b/packages/core/src/ItemWithParams/ItemWithParams.ts @@ -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 @@ -16,10 +16,10 @@ export const isItemWithParams = (item: ItemWithParams | unknown): item is ItemWi ) return true; return false; -} +}; export class ItemWithParams { - type = 'ItemWithParams' as const + type = 'ItemWithParams' as const; value: ExpectedType; params: Record; @@ -37,11 +37,11 @@ export class ItemWithParams { 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; } }, }); } -} \ No newline at end of file +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index f9aa9455..2d098390 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -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' \ No newline at end of file +export { Table } from './computers' diff --git a/packages/core/src/types/Errors.ts b/packages/core/src/types/Errors.ts new file mode 100644 index 00000000..c92f6c88 --- /dev/null +++ b/packages/core/src/types/Errors.ts @@ -0,0 +1,8 @@ +import { Node } from './Node'; + +export class NodeExecutionError extends Error { + constructor(message: string, public node: Pick) { + super(`Error in node ${node.label || node.id} (${node.type}): ${message}`); + this.name = 'NodeExecutionError'; + } +} diff --git a/packages/core/src/utils/withNodeExecutionErrorHandling.test.ts b/packages/core/src/utils/withNodeExecutionErrorHandling.test.ts new file mode 100644 index 00000000..b68b39f5 --- /dev/null +++ b/packages/core/src/utils/withNodeExecutionErrorHandling.test.ts @@ -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'); + }); +}); diff --git a/packages/core/src/utils/withNodeExecutionErrorHandling.ts b/packages/core/src/utils/withNodeExecutionErrorHandling.ts new file mode 100644 index 00000000..4c0aa9da --- /dev/null +++ b/packages/core/src/utils/withNodeExecutionErrorHandling.ts @@ -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( + runFn: (...args: TArgs) => AsyncGenerator, + node: Pick, +) { + return async function* (...args: TArgs): AsyncGenerator { + try { + return yield* runFn(...args); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + throw new NodeExecutionError(errorMessage, node); + } + }; +} From b8da3ac7c1e3fc9bee134e48f9a8d82a48e6dd84 Mon Sep 17 00:00:00 2001 From: roy Date: Fri, 15 Aug 2025 15:59:34 +0800 Subject: [PATCH 3/3] refactor(ExecutionMemoryFactory): Simplify error handling invocation --- packages/core/src/ExecutionMemoryFactory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/ExecutionMemoryFactory.ts b/packages/core/src/ExecutionMemoryFactory.ts index 0275f3dd..003eeb1f 100644 --- a/packages/core/src/ExecutionMemoryFactory.ts +++ b/packages/core/src/ExecutionMemoryFactory.ts @@ -65,7 +65,7 @@ export class ExecutionMemoryFactory { // Initialize runner context const context = new NodeRunnerContext(node.id); - context.status = withNodeExecutionErrorHandling(computer.run.bind(computer), node)({ + context.status = withNodeExecutionErrorHandling(computer.run, node)({ input: inputDevice, output: outputDevice, params: this.makeParamsDevice(node, memory),