From aa2ca8ac43d32e1552e3e19ae4efa2bd98531daf Mon Sep 17 00:00:00 2001 From: Zhang Yi Jiang Date: Tue, 18 Mar 2025 17:12:31 -0700 Subject: [PATCH 1/2] Add new error formatter --- packages/core/src/model/GraphProcessor.ts | 18 ++++----- packages/core/src/utils/errors.ts | 46 +++++++++++++++++++++++ packages/core/test/utils/errors.test.ts | 37 ++++++++++++++++++ 3 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 packages/core/test/utils/errors.test.ts diff --git a/packages/core/src/model/GraphProcessor.ts b/packages/core/src/model/GraphProcessor.ts index 464ea39e2..3f95a93ab 100644 --- a/packages/core/src/model/GraphProcessor.ts +++ b/packages/core/src/model/GraphProcessor.ts @@ -23,7 +23,7 @@ import type { GraphId, NodeGraph } from './NodeGraph.js'; import type { NodeImpl } from './NodeImpl.js'; import type { UserInputNode } from './nodes/UserInputNode.js'; import PQueueImport from 'p-queue'; -import { getError } from '../utils/errors.js'; +import { getError, NodeError } from '../utils/errors.js'; import Emittery from 'emittery'; import { entries, fromEntries, values } from '../utils/typeSafety.js'; import { isNotNull } from '../utils/genericUtilFunctions.js'; @@ -859,19 +859,19 @@ export class GraphProcessor { if (erroredNodes.length && !this.#abortSuccessfully) { let error = this.#abortError; if (!error) { - const message = `Graph ${this.#graph.metadata!.name} (${ - this.#graph.metadata!.id - }) failed to process due to errors in nodes:\n${erroredNodes - .map(([nodeId]) => `- ${this.#nodesById[nodeId]!.title} (${nodeId})`) - .join('\n')}`; + const { name, id } = this.#graph.metadata!; + const message = `Graph ${name} (${id}) failed to process due to errors in nodes`; if (erroredNodes.length === 1) { const [, nodeError] = erroredNodes[0]!; error = new Error(message, { cause: nodeError }); } else { - error = new AggregateError(erroredNodes.map(([, nodeError]) => nodeError), message); + error = new AggregateError( + erroredNodes.map(([, nodeError]) => nodeError), + message, + ); } } - + await this.#emitter.emit('graphError', { graph: this.#graph, error }); if (!this.#isSubProcessor) { @@ -1443,7 +1443,7 @@ export class GraphProcessor { // eslint-disable-next-line @typescript-eslint/no-floating-promises this.#emitter.emit('nodeError', { node, error, processId }); this.#emitTraceEvent(`Node ${node.title} (${node.id}-${processId}) errored: ${error.stack}`); - this.#erroredNodes.set(node.id, error); + this.#erroredNodes.set(node.id, new NodeError(error.message, node, { cause: error })); } getRootProcessor(): GraphProcessor { diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index 028ec6a07..af50c8162 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -1,3 +1,15 @@ +import type { ChartNode } from '../model/NodeBase.js'; + +export class NodeError extends Error { + constructor( + message: string, + public readonly node: ChartNode, + options?: ErrorOptions, + ) { + super(message, options); + } +} + /** Gets an Error from an unknown error object (strict unknown errors is enabled, helper util). */ export function getError(error: unknown): Error { const errorInstance = @@ -6,3 +18,37 @@ export function getError(error: unknown): Error { : new Error(error != null ? error.toString() : 'Unknown error'); return errorInstance; } + +export function rivetErrorToString(error: unknown, spaces = 0): string { + if (!(error instanceof Error)) { + if (error == null) { + return 'Unknown error'; + } + + return String(error); + } + + if (error instanceof AggregateError) { + return error.message + '\n' + error.errors.map((e) => ' - ' + rivetErrorToString(e, spaces + 4)).join('\n'); + } + + let message = error.message; + if (error instanceof NodeError) { + const { node } = error; + message = `${node.title} (${node.id}): ${error.message}`; + } + + if (error.cause) { + message += `\nCaused by: ${rivetErrorToString(error.cause)}`; + } + + return indent(message, spaces); +} + +function indent(str: string, spaces: number): string { + const spacing = ' '.repeat(spaces); + return str + .split('\n') + .map((line, i) => (i === 0 ? line : spacing + line)) + .join('\n'); +} diff --git a/packages/core/test/utils/errors.test.ts b/packages/core/test/utils/errors.test.ts new file mode 100644 index 000000000..fc7d9a07d --- /dev/null +++ b/packages/core/test/utils/errors.test.ts @@ -0,0 +1,37 @@ +import { it, describe } from 'node:test'; +import { strict as assert } from 'node:assert'; + +import { type NodeId } from '../../src/index.js'; +import { NodeError, rivetErrorToString } from '../../src/utils/errors.js'; + +describe('rivetErrorToString', () => { + it('should handle AggregateError', () => { + assert.equal( + rivetErrorToString( + new AggregateError( + [ + new Error('Error 1', { cause: new Error('Root cause') }), + new NodeError('Error 2', { + data: undefined, + id: 'nodeId' as NodeId, + title: 'Node title', + type: 'type', + visualData: {} as any, // Unused + }), + 'Error 3', + null, + ], + 'Multiple errors', + ), + ), + ` +Multiple errors + - Error 1 + Caused by: Root cause + - Node title (nodeId): Error 2 + - Error 3 + - Unknown error + `.trim(), + ); + }); +}); From 2bf6d033ec4d538646f8dd372bda1a7ed9feac58 Mon Sep 17 00:00:00 2001 From: Zhang Yi Jiang Date: Tue, 18 Mar 2025 17:41:10 -0700 Subject: [PATCH 2/2] Push node object into Error object --- packages/core/src/model/GraphProcessor.ts | 6 ++++-- packages/core/src/utils/errors.ts | 16 +++++++--------- packages/core/test/utils/errors.test.ts | 19 +++++++++++-------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/packages/core/src/model/GraphProcessor.ts b/packages/core/src/model/GraphProcessor.ts index 3f95a93ab..8567c0151 100644 --- a/packages/core/src/model/GraphProcessor.ts +++ b/packages/core/src/model/GraphProcessor.ts @@ -23,7 +23,8 @@ import type { GraphId, NodeGraph } from './NodeGraph.js'; import type { NodeImpl } from './NodeImpl.js'; import type { UserInputNode } from './nodes/UserInputNode.js'; import PQueueImport from 'p-queue'; -import { getError, NodeError } from '../utils/errors.js'; +import { getError } from '../utils/errors.js'; +import type { NodeError } from '../utils/errors.js'; import Emittery from 'emittery'; import { entries, fromEntries, values } from '../utils/typeSafety.js'; import { isNotNull } from '../utils/genericUtilFunctions.js'; @@ -1440,10 +1441,11 @@ export class GraphProcessor { #nodeErrored(node: ChartNode, e: unknown, processId: ProcessId) { const error = getError(e); + (error as NodeError).node = node; // eslint-disable-next-line @typescript-eslint/no-floating-promises this.#emitter.emit('nodeError', { node, error, processId }); this.#emitTraceEvent(`Node ${node.title} (${node.id}-${processId}) errored: ${error.stack}`); - this.#erroredNodes.set(node.id, new NodeError(error.message, node, { cause: error })); + this.#erroredNodes.set(node.id, error); } getRootProcessor(): GraphProcessor { diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index af50c8162..ef97758f5 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -1,13 +1,11 @@ import type { ChartNode } from '../model/NodeBase.js'; -export class NodeError extends Error { - constructor( - message: string, - public readonly node: ChartNode, - options?: ErrorOptions, - ) { - super(message, options); - } +export interface NodeError extends Error { + node: ChartNode; +} + +export function isNodeError(error: Error): error is NodeError { + return 'node' in error && typeof (error.node as ChartNode)?.id === 'string'; } /** Gets an Error from an unknown error object (strict unknown errors is enabled, helper util). */ @@ -33,7 +31,7 @@ export function rivetErrorToString(error: unknown, spaces = 0): string { } let message = error.message; - if (error instanceof NodeError) { + if (isNodeError(error)) { const { node } = error; message = `${node.title} (${node.id}): ${error.message}`; } diff --git a/packages/core/test/utils/errors.test.ts b/packages/core/test/utils/errors.test.ts index fc7d9a07d..f429f20d6 100644 --- a/packages/core/test/utils/errors.test.ts +++ b/packages/core/test/utils/errors.test.ts @@ -6,18 +6,21 @@ import { NodeError, rivetErrorToString } from '../../src/utils/errors.js'; describe('rivetErrorToString', () => { it('should handle AggregateError', () => { + const nodeError = new Error('Error 2'); + (nodeError as NodeError).node = { + data: undefined, + id: 'nodeId' as NodeId, + title: 'Node title', + type: 'type', + visualData: {} as any, // Unused + }; + assert.equal( rivetErrorToString( new AggregateError( [ - new Error('Error 1', { cause: new Error('Root cause') }), - new NodeError('Error 2', { - data: undefined, - id: 'nodeId' as NodeId, - title: 'Node title', - type: 'type', - visualData: {} as any, // Unused - }), + new Error('Error 1', { cause: new Error('Root cause') }), // + nodeError, 'Error 3', null, ],