fix(core): Preserve cause on execution errors (#35216)

packages/core/src/errors/__tests__/error-reporter.test.ts

@@ -389,6 +389,16 @@ describe('ErrorReporter', () => {
 			expect(logger.error).toHaveBeenNthCalledWith(1, `outer\n${outer.stack}\n`, undefined);
 			expect(logger.error).toHaveBeenNthCalledWith(2, `root cause\n${cause.stack}\n`, undefined);
 		});
+
+		it('should stop walking a cause chain that loops back on itself', () => {
+			const outer = new Error('outer');
+			const inner = new Error('inner', { cause: outer });
+			outer.cause = inner;
+
+			errorReporter.error(outer);
+
+			expect(logger.error).toHaveBeenCalledTimes(2);
+		});
 	});
 
 	describe('error with Sentry initialized', () => {

packages/core/src/errors/error-reporter.ts

@@ -130,7 +130,11 @@ export class ErrorReporter {
 			const { executionId } = options ?? {};
 			const context = executionId ? ` (execution ${executionId})` : '';
 
+			// Prevent infinite loops in cyclic cause chains.
+			const seen = new Set<Error>();
+
 			do {
+				seen.add(e);
 				let stack = '';
 				let meta = undefined;
 				if (e instanceof ApplicationError || e instanceof BaseError) {
@@ -147,7 +151,7 @@ export class ErrorReporter {
 					this.logger.error(msg, meta);
 				}
 				e = e.cause as Error;
-			} while (e);
+			} while (e && !seen.has(e));
 		}
 	}
 

packages/core/src/execution-engine/node-execution-context/__tests__/execute-context.test.ts

@@ -177,6 +177,7 @@ describe('ExecuteContext', () => {
 
 			expect(() => executeContext.getNodeParameter('testParameter', 0)).toThrow(error);
 			expect(error.context.parameter).toEqual('testParameter');
+			expect({ ...error }).toEqual(expect.objectContaining({ cause: 'testValue' }));
 		});
 
 		it('should handle expression errors on Set nodes (Ticket #PAY-684)', () => {

packages/core/src/execution-engine/node-execution-context/node-execution-context.ts

@@ -544,8 +544,12 @@ export abstract class NodeExecutionContext implements Omit<FunctionsBase, 'getCr
 			} else {
 				// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
 				if (e.context) e.context.parameter = parameterName;
-				// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
-				e.cause = value;
+				Object.defineProperty(e, 'cause', {
+					value,
+					writable: true,
+					enumerable: true,
+					configurable: true,
+				});
 				throw e;
 			}
 		}

packages/workflow/src/errors/abstract/execution-base.error.ts

@@ -7,12 +7,13 @@ interface ExecutionBaseErrorOptions extends BaseErrorOptions {
 }
 
 export abstract class ExecutionBaseError extends BaseError {
-	// `BaseError.description` is readonly, but subclasses (e.g. NodeApiError,
-	// ExpressionError) reassign it, so redeclare it as writable here. `declare`
-	// avoids re-initializing the field and clobbering the value set by `super`.
+	// `declare` is load-bearing on both fields: at target es2022 a plain
+	// redeclaration is defined as `undefined` after `super()` runs, clobbering what
+	// `super` set — `description` from `BaseError`, `cause` from native `Error`.
+	// `description` is readonly on `BaseError`, but subclasses reassign it.
 	declare description: string | null | undefined;
 
-	override cause?: Error;
+	declare cause?: Error;
 
 	errorResponse?: JsonObject;
 
@@ -36,11 +37,11 @@ export abstract class ExecutionBaseError extends BaseError {
 		});
 		this.timestamp = Date.now();
 
+		// `cause` itself needs no assignment: `BaseError` forwards it to the native
+		// `Error` constructor, which installs it as a non-enumerable own property.
 		const { cause, errorResponse } = options;
 		if (cause instanceof ExecutionBaseError) {
 			this.context = cause.context;
-		} else if (cause && !(cause instanceof Error)) {
-			this.cause = cause;
 		}
 
 		if (errorResponse) this.errorResponse = errorResponse;
@@ -54,7 +55,12 @@ export abstract class ExecutionBaseError extends BaseError {
 			name: this.name,
 			description: this.description,
 			context: this.context,
-			cause: this.cause,
+			// Bounded projection: an `Error` cause can be a raw axios error carrying
+			// request headers and circular refs, and this output is persisted.
+			cause:
+				this.cause instanceof Error
+					? { name: this.cause.name, message: this.cause.message }
+					: this.cause,
 		};
 	}
 }

packages/workflow/src/errors/abstract/node.error.ts

@@ -79,7 +79,11 @@ export abstract class NodeError extends ExecutionBaseError {
 		jsonError: JsonObject,
 		potentialKeys: string[],
 		traversalKeys: string[] = [],
+		ancestors = new Set<JsonObject>(),
 	): string | null {
+		if (ancestors.has(jsonError)) return null;
+		const seen = new Set(ancestors).add(jsonError);
+
 		for (const key of potentialKeys) {
 			let value = jsonError[key];
 			if (value) {
@@ -99,7 +103,7 @@ export abstract class NodeError extends ExecutionBaseError {
 							if (typeof jsonError === 'string') return jsonError;
 							if (typeof jsonError === 'number') return jsonError.toString();
 							if (isTraversableObject(jsonError)) {
-								return this.findProperty(jsonError, potentialKeys);
+								return this.findProperty(jsonError, potentialKeys, [], seen);
 							}
 							return null;
 						})
@@ -111,7 +115,7 @@ export abstract class NodeError extends ExecutionBaseError {
 					return resolvedErrors.join(' | ');
 				}
 				if (isTraversableObject(value)) {
-					const property = this.findProperty(value, potentialKeys);
+					const property = this.findProperty(value, potentialKeys, [], seen);
 					if (property) {
 						return property;
 					}
@@ -122,7 +126,7 @@ export abstract class NodeError extends ExecutionBaseError {
 		for (const key of traversalKeys) {
 			const value = jsonError[key];
 			if (isTraversableObject(value)) {
-				const property = this.findProperty(value, potentialKeys, traversalKeys);
+				const property = this.findProperty(value, potentialKeys, traversalKeys, seen);
 				if (property) {
 					return property;
 				}

packages/workflow/test/errors/execution-base.error.test.ts

@@ -1,12 +1,88 @@
-import { ExpressionError } from '../../src/errors';
+import { mock } from 'vitest-mock-extended';
+
+import {
+	ExpressionError,
+	NodeApiError,
+	NodeOperationError,
+	NodeSslError,
+	WorkflowOperationError,
+} from '../../src/errors';
+import type { INode, JsonObject } from '../../src/interfaces';
+
+/**
+ * Shaped like the axios errors the request helpers pass to `NodeApiError`. Typed
+ * as `JsonObject` too because that is what `NodeApiError` declares, even though
+ * every real call site hands it an `Error`.
+ */
+const axiosError = () =>
+	Object.assign(new Error('Request failed with status code 403'), {
+		name: 'AxiosError',
+		isAxiosError: true,
+		config: { headers: { authorization: 'Bearer secret-token' } },
+		response: { status: 403, data: { error: 'insufficient_scope' } },
+	}) as unknown as Error & JsonObject;
 
 describe('ExecutionBaseError', () => {
+	const node = mock<INode>();
+
 	it('should set name to the concrete class name', () => {
 		const error = new ExpressionError('message');
 
 		expect(error.name).toBe('ExpressionError');
 	});
 
+	describe('cause', () => {
+		it.each([
+			['NodeApiError', (cause: Error & JsonObject) => new NodeApiError(node, cause)],
+			['NodeOperationError', (cause: Error & JsonObject) => new NodeOperationError(node, cause)],
+			['ExpressionError', (cause: Error & JsonObject) => new ExpressionError('message', { cause })],
+			['NodeSslError', (cause: Error & JsonObject) => new NodeSslError(cause)],
+		])('should preserve an Error cause on %s', (_name, construct) => {
+			const cause = axiosError();
+
+			expect(construct(cause).cause).toBe(cause);
+		});
+
+		it('should keep cause non-enumerable so it stays out of serialized execution data', () => {
+			const cause = axiosError();
+			const error = new NodeApiError(node, cause);
+
+			// `workflow-execute.ts` persists node errors as `{ ...error }`, which copies
+			// own enumerable properties only. Keeping the native descriptor is what stops
+			// the raw axios error - including `config.headers.authorization` - from being
+			// written to the database, to error workflows and to the UI.
+			expect(Object.getOwnPropertyDescriptor(error, 'cause')?.enumerable).toBe(false);
+			expect(JSON.stringify({ ...error })).not.toContain('secret-token');
+		});
+
+		it('should adopt the context of an ExecutionBaseError cause and retain the cause', () => {
+			const cause = new NodeApiError(node, axiosError());
+			const error = new NodeOperationError(node, cause);
+
+			expect(error.cause).toBe(cause);
+			expect(error.context).toBe(cause.context);
+		});
+	});
+
+	describe('toJSON', () => {
+		it('should reduce an Error cause to its name and message', () => {
+			const error = new NodeApiError(node, axiosError());
+
+			expect(error.toJSON!().cause).toEqual({
+				name: 'AxiosError',
+				message: 'Request failed with status code 403',
+			});
+		});
+
+		it('should pass through a non-Error cause unchanged', () => {
+			const error = new WorkflowOperationError('message');
+			const cause = { failed: 'parameter' };
+			error.cause = cause as unknown as Error;
+
+			expect(error.toJSON!().cause).toBe(cause);
+		});
+	});
+
 	describe('with a frozen Error.prototype', () => {
 		const nameDescriptor = Object.getOwnPropertyDescriptor(Error.prototype, 'name')!;
 

packages/workflow/test/errors/node.error.test.ts

@@ -2,7 +2,7 @@ import { mock } from 'vitest-mock-extended';
 
 import { NodeApiError } from '../../src/errors/node-api.error';
 import { NodeOperationError } from '../../src/errors/node-operation.error';
-import type { INode } from '../../src/interfaces';
+import type { INode, JsonObject } from '../../src/interfaces';
 
 describe('NodeError', () => {
 	const node = mock<INode>();
@@ -21,4 +21,15 @@ describe('NodeError', () => {
 
 		vi.useRealTimers();
 	});
+
+	it('should find an error message when the cause is circular', () => {
+		const errorResponse = Object.assign(new Error('', { cause: undefined }), {
+			response: { data: { message: 'Some error happened' } },
+		}) as unknown as Error & JsonObject;
+		errorResponse.cause = errorResponse;
+
+		const error = new NodeApiError(node, errorResponse);
+
+		expect(error.message).toBe('Some error happened');
+	});
 });