fix(HTTP Request Node): Cleanup circular references in response (#6590)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>

packages/nodes-base/nodes/HttpRequest/V1/HttpRequestV1.node.ts

@@ -8,12 +8,11 @@ import type {
 	INodeTypeDescription,
 	JsonObject,
 } from 'n8n-workflow';
-import { NodeApiError, NodeOperationError, sleep } from 'n8n-workflow';
+import { NodeApiError, NodeOperationError, sleep, removeCircularRefs } from 'n8n-workflow';
 
 import type { OptionsWithUri } from 'request';
 import type { IAuthDataSanitizeKeys } from '../GenericFunctions';
 import { replaceNullValues, sanitizeUiMessage } from '../GenericFunctions';
-
 interface OptionData {
 	name: string;
 	displayName: string;
@@ -976,6 +975,7 @@ export class HttpRequestV1 implements INodeType {
 					// throw error;
 					throw new NodeApiError(this.getNode(), response as JsonObject, { itemIndex });
 				} else {
+					removeCircularRefs(response.reason as JsonObject);
 					// Return the actual reason as error
 					returnItems.push({
 						json: {

packages/nodes-base/nodes/HttpRequest/V2/HttpRequestV2.node.ts

@@ -7,7 +7,7 @@ import type {
 	INodeTypeDescription,
 	JsonObject,
 } from 'n8n-workflow';
-import { NodeApiError, NodeOperationError, sleep } from 'n8n-workflow';
+import { NodeApiError, NodeOperationError, sleep, removeCircularRefs } from 'n8n-workflow';
 
 import type { OptionsWithUri } from 'request';
 import type { IAuthDataSanitizeKeys } from '../GenericFunctions';
@@ -1022,12 +1022,12 @@ export class HttpRequestV2 implements INodeType {
 		for (let itemIndex = 0; itemIndex < items.length; itemIndex++) {
 			// @ts-ignore
 			response = promisesResponses.shift();
-
 			if (response!.status !== 'fulfilled') {
 				if (!this.continueOnFail()) {
 					// throw error;
 					throw new NodeApiError(this.getNode(), response as JsonObject, { itemIndex });
 				} else {
+					removeCircularRefs(response.reason as JsonObject);
 					// Return the actual reason as error
 					returnItems.push({
 						json: {

packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts

@@ -12,7 +12,16 @@ import type {
 	JsonObject,
 } from 'n8n-workflow';
 
-import { BINARY_ENCODING, jsonParse, NodeApiError, NodeOperationError, sleep } from 'n8n-workflow';
+import {
+	BINARY_ENCODING,
+	jsonParse,
+	NodeApiError,
+	NodeOperationError,
+	sleep,
+	removeCircularRefs,
+} from 'n8n-workflow';
+
+import { keysToLowercase } from '@utils/utilities';
 
 import type { OptionsWithUri } from 'request-promise-native';
 
@@ -25,7 +34,6 @@ import {
 	replaceNullValues,
 	sanitizeUiMessage,
 } from '../GenericFunctions';
-import { keysToLowercase } from '@utils/utilities';
 
 function toText<T>(data: T) {
 	if (typeof data === 'object' && data !== null) {
@@ -1428,6 +1436,7 @@ export class HttpRequestV3 implements INodeType {
 					}
 					throw new NodeApiError(this.getNode(), response as JsonObject, { itemIndex });
 				} else {
+					removeCircularRefs(response.reason as JsonObject);
 					// Return the actual reason as error
 					returnItems.push({
 						json: {

packages/workflow/src/Expression.ts

@@ -14,15 +14,30 @@ import type {
 	NodeParameterValueType,
 	WorkflowExecuteMode,
 } from './Interfaces';
-import { ExpressionError } from './ExpressionError';
+import { ExpressionError, ExpressionExtensionError } from './ExpressionError';
 import { WorkflowDataProxy } from './WorkflowDataProxy';
 import type { Workflow } from './Workflow';
 
-// eslint-disable-next-line import/no-cycle
 import { extend, extendOptional } from './Extensions';
 import { extendedFunctions } from './Extensions/ExtendedFunctions';
 import { extendSyntax } from './Extensions/ExpressionExtension';
-import { isExpressionError, IS_FRONTEND, isSyntaxError, isTypeError } from './utils';
+
+const IS_FRONTEND_IN_DEV_MODE =
+	typeof process === 'object' &&
+	Object.keys(process).length === 1 &&
+	'env' in process &&
+	Object.keys(process.env).length === 0;
+
+const IS_FRONTEND = typeof process === 'undefined' || IS_FRONTEND_IN_DEV_MODE;
+
+export const isSyntaxError = (error: unknown): error is SyntaxError =>
+	error instanceof SyntaxError || (error instanceof Error && error.name === 'SyntaxError');
+
+export const isExpressionError = (error: unknown): error is ExpressionError =>
+	error instanceof ExpressionError || error instanceof ExpressionExtensionError;
+
+export const isTypeError = (error: unknown): error is TypeError =>
+	error instanceof TypeError || (error instanceof Error && error.name === 'TypeError');
 
 // Set it to use double curly brackets instead of single ones
 tmpl.brackets.set('{{ }}');

packages/workflow/src/NodeErrors.ts

@@ -7,6 +7,7 @@
 /* eslint-disable @typescript-eslint/no-unsafe-assignment */
 // eslint-disable-next-line max-classes-per-file
 import { parseString } from 'xml2js';
+import { removeCircularRefs, isTraversableObject } from './utils';
 import type { IDataObject, INode, IStatusCodeMessages, JsonObject } from './Interfaces';
 
 type Severity = 'warning' | 'error';
@@ -156,7 +157,7 @@ export abstract class NodeError extends ExecutionBaseError {
 						.map((jsonError) => {
 							if (typeof jsonError === 'string') return jsonError;
 							if (typeof jsonError === 'number') return jsonError.toString();
-							if (this.isTraversableObject(jsonError)) {
+							if (isTraversableObject(jsonError)) {
 								return this.findProperty(jsonError, potentialKeys);
 							}
 							return null;
@@ -168,7 +169,7 @@ export abstract class NodeError extends ExecutionBaseError {
 					}
 					return resolvedErrors.join(' | ');
 				}
-				if (this.isTraversableObject(value)) {
+				if (isTraversableObject(value)) {
 					const property = this.findProperty(value, potentialKeys);
 					if (property) {
 						return property;
@@ -180,7 +181,7 @@ export abstract class NodeError extends ExecutionBaseError {
 		// eslint-disable-next-line no-restricted-syntax
 		for (const key of traversalKeys) {
 			const value = jsonError[key];
-			if (this.isTraversableObject(value)) {
+			if (isTraversableObject(value)) {
 				const property = this.findProperty(value, potentialKeys, traversalKeys);
 				if (property) {
 					return property;
@@ -190,47 +191,6 @@ export abstract class NodeError extends ExecutionBaseError {
 
 		return null;
 	}
-
-	/**
-	 * Check if a value is an object with at least one key, i.e. it can be traversed.
-	 */
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any
-	protected isTraversableObject(value: any): value is JsonObject {
-		return (
-			value &&
-			typeof value === 'object' &&
-			!Array.isArray(value) &&
-			// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-			!!Object.keys(value).length
-		);
-	}
-
-	/**
-	 * Remove circular references from objects.
-	 */
-	protected removeCircularRefs(obj: JsonObject, seen = new Set()) {
-		seen.add(obj);
-		Object.entries(obj).forEach(([key, value]) => {
-			if (this.isTraversableObject(value)) {
-				// eslint-disable-next-line @typescript-eslint/no-unused-expressions
-				seen.has(value)
-					? (obj[key] = { circularReference: true })
-					: this.removeCircularRefs(value, seen);
-				return;
-			}
-			if (Array.isArray(value)) {
-				value.forEach((val, index) => {
-					if (seen.has(val)) {
-						value[index] = { circularReference: true };
-						return;
-					}
-					if (this.isTraversableObject(val)) {
-						this.removeCircularRefs(val, seen);
-					}
-				});
-			}
-		});
-	}
 }
 
 interface NodeOperationErrorOptions {
@@ -317,7 +277,7 @@ export class NodeApiError extends NodeError {
 
 		if (error.error) {
 			// only for request library error
-			this.removeCircularRefs(error.error as JsonObject);
+			removeCircularRefs(error.error as JsonObject);
 		}
 
 		if ((!message && (error.message || (error?.reason as IDataObject)?.message)) || description) {

packages/workflow/src/index.ts

@@ -31,6 +31,7 @@ export {
 	sleep,
 	fileTypeFromMimeType,
 	assert,
+	removeCircularRefs,
 } from './utils';
 export {
 	isINodeProperties,

packages/workflow/src/utils.ts

@@ -1,5 +1,4 @@
-import { ExpressionError, ExpressionExtensionError } from './ExpressionError';
+import type { BinaryFileType, JsonObject } from './Interfaces';
-import type { BinaryFileType } from './Interfaces';
 
 const readStreamClasses = new Set(['ReadStream', 'Readable', 'ReadableStream']);
 
@@ -129,19 +128,34 @@ export function assert<T>(condition: T, msg?: string): asserts condition {
 	}
 }
 
-const IS_FRONTEND_IN_DEV_MODE =
+export const isTraversableObject = (value: any): value is JsonObject => {
-	typeof process === 'object' &&
+	return (
-	Object.keys(process).length === 1 &&
+		value &&
-	'env' in process &&
+		typeof value === 'object' &&
-	Object.keys(process.env).length === 0;
+		!Array.isArray(value) &&
+		// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
+		!!Object.keys(value).length
+	);
+};
 
-export const IS_FRONTEND = typeof process === 'undefined' || IS_FRONTEND_IN_DEV_MODE;
+export const removeCircularRefs = (obj: JsonObject, seen = new Set()) => {
-
+	seen.add(obj);
-export const isSyntaxError = (error: unknown): error is SyntaxError =>
+	Object.entries(obj).forEach(([key, value]) => {
-	error instanceof SyntaxError || (error instanceof Error && error.name === 'SyntaxError');
+		if (isTraversableObject(value)) {
-
+			// eslint-disable-next-line @typescript-eslint/no-unused-expressions
-export const isExpressionError = (error: unknown): error is ExpressionError =>
+			seen.has(value) ? (obj[key] = { circularReference: true }) : removeCircularRefs(value, seen);
-	error instanceof ExpressionError || error instanceof ExpressionExtensionError;
+			return;
-
+		}
-export const isTypeError = (error: unknown): error is TypeError =>
+		if (Array.isArray(value)) {
-	error instanceof TypeError || (error instanceof Error && error.name === 'TypeError');
+			value.forEach((val, index) => {
+				if (seen.has(val)) {
+					value[index] = { circularReference: true };
+					return;
+				}
+				if (isTraversableObject(val)) {
+					removeCircularRefs(val, seen);
+				}
+			});
+		}
+	});
+};