fix: Stop replacing error messages with "Unknown error" (#36820)

Co-authored-by: n8n-cat-bot[bot] <n8n-cat-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts

@@ -1343,6 +1343,28 @@ describe('JsTaskRunner', () => {
 			},
 		);
 
+		// A database driver error, e.g. `pg`'s `DatabaseError`, sets `name` to
+		// `'error'`, so V8 renders the stack header lowercased as `error: <message>`.
+		// The message the database gave must survive to the user.
+		test.each<[CodeExecutionMode]>([['runOnceForAllItems'], ['runOnceForEachItem']])(
+			'should surface the message of a rethrown database error in %s mode',
+			async (nodeMode) => {
+				const code = `
+					const dbError = new Error('duplicate key value violates unique constraint "users_email_key"');
+					dbError.name = 'error';
+					dbError.code = '23505';
+					throw dbError;
+				`;
+
+				await expect(
+					execTaskWithParams({
+						task: newTaskParamsWithSettings({ code, nodeMode }),
+						taskData: newDataRequestResponse([wrapIntoJson({ a: 1 })]),
+					}),
+				).rejects.toThrow('duplicate key value violates unique constraint "users_email_key"');
+			},
+		);
+
 		it('sends serializes an error correctly', async () => {
 			const runner = createRunnerWithOpts({});
 			const taskId = '1';

packages/@n8n/task-runner/src/js-task-runner/errors/__tests__/execution-error.test.ts

@@ -34,6 +34,184 @@ describe('ExecutionError', () => {
 		expect(executionError.context).toEqual({ itemIndex: 1 });
 	});
 
+	it('should parse a stack with CRLF line endings', () => {
+		const error = new Error('a.unknown is not a function');
+		Object.defineProperty(error, 'stack', { value: defaultStack.replace(/\n/g, '\r\n') });
+
+		const executionError = new ExecutionError(error);
+
+		expect(executionError.message).toBe('a.unknown is not a function [line 2]');
+		expect(executionError.description).toBe('TypeError');
+	});
+
+	// A database driver error, e.g. `pg`'s `DatabaseError`, sets `name` to
+	// `'error'`, so V8 renders the stack header lowercased as `error: <message>`.
+	describe('errors whose stack header is not "Error:"', () => {
+		it('should keep the message of a driver error thrown outside the sandbox', () => {
+			const driverStack = `error: duplicate key value violates unique constraint "users_email_key"
+    at Parser.parseErrorMessage (/n8n/node_modules/pg-protocol/dist/parser.js:283:98)
+    at Parser.handlePacket (/n8n/node_modules/pg-protocol/dist/parser.js:122:29)
+    at Parser.parse (/n8n/node_modules/pg-protocol/dist/parser.js:35:38)
+    at Socket.<anonymous> (/n8n/node_modules/pg-protocol/dist/index.js:11:42)
+    at Socket.emit (node:events:518:28)
+    at addChunk (node:internal/streams/readable:561:12)
+    at TCP.onStreamRead (node:internal/stream_base_commons:191:23)`;
+
+			const error = new Error('duplicate key value violates unique constraint "users_email_key"');
+			error.name = 'error';
+			Object.defineProperty(error, 'stack', { value: driverStack });
+
+			const executionError = new ExecutionError(error);
+
+			expect(executionError.message).toBe(
+				'duplicate key value violates unique constraint "users_email_key"',
+			);
+		});
+
+		it('should keep the message of a driver error rethrown from user code', () => {
+			const driverStack = `error: null value in column "email" of relation "users" violates not-null constraint
+    at VmCodeWrapper (evalmachine.<anonymous>:4:9)
+    at evalmachine.<anonymous>:7:2
+    at Script.runInContext (node:vm:149:12)
+    at runInContext (node:vm:301:6)
+    at JsTaskRunner.runForAllItems (/n8n/packages/@n8n/task-runner/dist/js-task-runner/js-task-runner.js:90:65)
+    at JsTaskRunner.executeTask (/n8n/packages/@n8n/task-runner/dist/js-task-runner/js-task-runner.js:71:26)`;
+
+			const error = new Error(
+				'null value in column "email" of relation "users" violates not-null constraint',
+			);
+			error.name = 'error';
+			Object.defineProperty(error, 'stack', { value: driverStack });
+
+			const executionError = new ExecutionError(error, 0);
+
+			expect(executionError.message).toBe(
+				'null value in column "email" of relation "users" violates not-null constraint [line 4, for item 0]',
+			);
+			expect(executionError.lineNumber).toBe(4);
+		});
+
+		it.each([
+			['a name containing a space', 'Database Error'],
+			['a name containing a dot', 'my.CustomError'],
+			['a name containing a dash', 'Custom-Error'],
+			['a name carrying a node error code', 'TypeError [ERR_INVALID_ARG_TYPE]'],
+		])('should keep the message and surface the type for %s', (_, errorType) => {
+			const error = new Error('oops');
+			error.name = errorType;
+			Object.defineProperty(error, 'stack', {
+				value: `${errorType}: oops\n    at VmCodeWrapper (evalmachine.<anonymous>:2:9)`,
+			});
+
+			const executionError = new ExecutionError(error);
+
+			expect(executionError.message).toBe('oops [line 2]');
+			expect(executionError.description).toBe(errorType);
+		});
+	});
+
+	describe('messages containing ": "', () => {
+		// Previously handled by two hard-coded special cases that stitched a message
+		// split on every colon back together.
+		it.each([
+			"Cannot find module 'node:fs'",
+			"Module 'node:fs' is disallowed",
+			"Module 'node:child_process' is disallowed",
+		])('should keep "%s" whole', (message) => {
+			const error = new Error(message);
+			Object.defineProperty(error, 'stack', {
+				value: `Error: ${message}\n    at VmCodeWrapper (evalmachine.<anonymous>:2:9)`,
+			});
+
+			const executionError = new ExecutionError(error);
+
+			expect(executionError.message).toBe(`${message} [line 2]`);
+			expect(executionError.description).toBeNull();
+		});
+
+		// Deliberate change: the message used to be split on every colon, so this
+		// surfaced as message "Not Found" with description "Request failed with
+		// status code 404" - the leading half of the message became the type.
+		it('should not split a message on a colon it happens to contain', () => {
+			const message = 'Request failed with status code 404: Not Found';
+			const error = new Error(message);
+			Object.defineProperty(error, 'stack', {
+				value: `Error: ${message}\n    at VmCodeWrapper (evalmachine.<anonymous>:2:9)`,
+			});
+
+			const executionError = new ExecutionError(error);
+
+			expect(executionError.message).toBe(`${message} [line 2]`);
+			expect(executionError.description).toBeNull();
+		});
+
+		it('should not mistake a continuation row of a multi-line message for the header', () => {
+			const error = new Error('request failed\ncause: ETIMEDOUT');
+			Object.defineProperty(error, 'stack', {
+				value: `Error: request failed
+cause: ETIMEDOUT
+    at VmCodeWrapper (evalmachine.<anonymous>:2:9)`,
+			});
+
+			const executionError = new ExecutionError(error);
+
+			expect(executionError.message).toBe('request failed [line 2]');
+			expect(executionError.description).toBeNull();
+		});
+	});
+
+	// V8 prefixes a syntax error's stack with an excerpt of the offending source,
+	// closed by a caret row. The excerpt holds user code, so a row of it can look
+	// like a stack header.
+	it('should skip the source excerpt prefixing a syntax error stack', () => {
+		const syntaxErrorStack = `evalmachine.<anonymous>:2
+data: { foo: 1
+             ^
+
+SyntaxError: Unexpected end of input
+    at new Script (node:vm:117:7)
+    at createScript (node:vm:269:10)
+    at Object.runInNewContext (node:vm:310:10)`;
+
+		const error = new SyntaxError('Unexpected end of input');
+		Object.defineProperty(error, 'stack', { value: syntaxErrorStack });
+
+		const executionError = new ExecutionError(error);
+
+		expect(executionError.message).toBe('Unexpected end of input');
+		expect(executionError.description).toBe('SyntaxError');
+	});
+
+	describe('stacks without a parseable header', () => {
+		it("should keep the error's own message", () => {
+			const error = new Error('connect ETIMEDOUT 10.0.0.1:5432');
+			Object.defineProperty(error, 'stack', {
+				value: `    at VmCodeWrapper (evalmachine.<anonymous>:3:9)
+    at evalmachine.<anonymous>:7:2`,
+			});
+
+			const executionError = new ExecutionError(error);
+
+			expect(executionError.message).toBe('connect ETIMEDOUT 10.0.0.1:5432 [line 3]');
+			expect(executionError.description).toBeNull();
+			expect(executionError.lineNumber).toBe(3);
+		});
+
+		it("should keep the error's own message when there is no stack at all", () => {
+			const executionError = new ExecutionError({ message: '{"code":"ECONNRESET"}' });
+
+			expect(executionError.message).toBe('{"code":"ECONNRESET"}');
+			expect(executionError.description).toBeNull();
+		});
+
+		it('should fall back to "Unknown error" when there is no message either', () => {
+			const executionError = new ExecutionError({ message: '' });
+
+			expect(executionError.message).toBe('Unknown error');
+			expect(executionError.description).toBeNull();
+		});
+	});
+
 	it('should serialize correctly', () => {
 		const error = new Error('a.unknown is not a function');
 		Object.defineProperty(error, 'stack', {

packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts

@@ -1,6 +1,25 @@
 import type { ErrorLike } from './error-like';
 import { SerializableError } from './serializable-error';
 
+/**
+ * Matches the header row of a stack, i.e. `<error name>: <error message>`. The
+ * name is whatever the thrown error's `name` is, which is not always
+ * `Error`-suffixed - `pg`'s `DatabaseError` sets it to `error`, and a custom
+ * error may contain spaces, dots or dashes - so accept any name up to the first
+ * `': '`, instead of looking for an `Error:` prefix.
+ */
+const STACK_HEADER_REGEX = /^(?<errorType>\S.*?): (?<errorDetails>.+)$/;
+
+/** Matches a stack frame row, e.g. `    at Object.foo (/a/b.js:1:2)`. */
+const STACK_FRAME_REGEX = /^\s+at\s/;
+
+/**
+ * Matches the caret row that closes the source excerpt V8 prefixes a syntax
+ * error's stack with. The excerpt holds user code, which can look like a header
+ * row, so the header is only searched for past the caret.
+ */
+const SOURCE_EXCERPT_CARET_REGEX = /^\s*\^+\s*$/;
+
 export class ExecutionError extends SerializableError {
 	description: string | null = null;
 
@@ -29,34 +48,40 @@ export class ExecutionError extends SerializableError {
 	}
 
 	/**
-	 * Populate error `message` and `description` from error `stack`.
+	 * Populate error `message` and `description` from error `stack`. The stack is
+	 * the richer source - it carries the error name and the line number - but the
+	 * error's own `message` is always present, so keep it as the fallback for a
+	 * stack we cannot parse, rather than discarding it.
 	 */
 	private populateFromStack() {
-		const stackRows = (this.stack ?? '').split('\n');
+		const fallbackMessage = this.message || 'Unknown error';
+		const stackRows = (this.stack ?? '').split(/\r?\n/);
 
-		if (stackRows.length === 0) {
-			this.message = 'Unknown error';
-			return;
-		}
-
-		const messageRow = stackRows.find((line) => line.includes('Error:'));
 		const lineNumberDisplay = this.toLineNumberDisplay(stackRows);
-
-		if (!messageRow) {
-			this.message = `Unknown error ${lineNumberDisplay}`;
-			return;
-		}
-
-		const [errorDetails, errorType] = this.toErrorDetailsAndType(messageRow);
+		const [errorDetails, errorType] = this.toErrorDetailsAndType(this.toMessageRow(stackRows));
 
 		if (errorType) this.description = errorType;
 
-		if (!errorDetails) {
-			this.message = `Unknown error ${lineNumberDisplay}`;
-			return;
-		}
+		this.message = `${errorDetails ?? fallbackMessage} ${lineNumberDisplay}`.trim();
+	}
 
-		this.message = `${errorDetails} ${lineNumberDisplay}`;
+	/**
+	 * Find the row holding `<error name>: <error message>`, i.e. the first row
+	 * past any source excerpt and before the first stack frame. Rows after it can
+	 * be continuation rows of a multi-line message, which must not be mistaken
+	 * for the header.
+	 */
+	private toMessageRow(stackRows: string[]) {
+		const firstFrameIndex = stackRows.findIndex((row) => STACK_FRAME_REGEX.test(row));
+		const headerRows = firstFrameIndex === -1 ? stackRows : stackRows.slice(0, firstFrameIndex);
+
+		const excerptEndIndex = headerRows.reduce(
+			(lastCaretIndex, row, index) =>
+				SOURCE_EXCERPT_CARET_REGEX.test(row) ? index + 1 : lastCaretIndex,
+			0,
+		);
+
+		return headerRows.slice(excerptEndIndex).find((row) => STACK_HEADER_REGEX.test(row));
 	}
 
 	private toLineNumberDisplay(stackRows: string[]) {
@@ -90,24 +115,13 @@ export class ExecutionError extends SerializableError {
 	}
 
 	private toErrorDetailsAndType(messageRow?: string) {
-		if (!messageRow) return [null, null];
+		const groups = messageRow?.match(STACK_HEADER_REGEX)?.groups;
 
-		const segments = messageRow.split(':').map((i) => i.trim());
-		if (segments[1] === "Cannot find module 'node") {
-			segments[1] = `${segments[1]}:${segments[2]}`;
-			segments.splice(2, 1);
-		}
+		if (!groups) return [null, null];
 
-		if (
-			segments.length >= 3 &&
-			segments[1]?.startsWith("Module 'node") &&
-			segments[2]?.includes("' is disallowed")
-		) {
-			segments[1] = `${segments[1]}:${segments[2]}`;
-			segments.splice(2, 1);
-		}
+		const { errorType, errorDetails } = groups;
 
-		const [errorDetails, errorType] = segments.reverse();
-		return [errorDetails, errorType === 'Error' ? null : errorType];
+		// `Error`, and driver names like `error`, tell the user nothing
+		return [errorDetails.trim() || null, errorType.toLowerCase() === 'error' ? null : errorType];
 	}
 }