fix(core): Let LLM failed-attempt handling retry and honour Retry-After (#41002)

packages/@n8n/ai-utilities/src/__tests__/utils/failed-attempt-handler/n8nDefaultFailedAttemptHandler.test.ts

@@ -63,4 +63,72 @@ describe('n8nDefaultFailedAttemptHandler', () => {
 		const error = new Error('Some random error');
 		expect(() => n8nDefaultFailedAttemptHandler(error)).not.toThrow();
 	});
+
+	it('should not retry a 413', () => {
+		const error = new MockHttpError('Too large', 413);
+		expect(() => n8nDefaultFailedAttemptHandler(error)).toThrow(error);
+	});
+
+	describe('429', () => {
+		it.each([
+			[
+				'top-level code',
+				Object.assign(new Error('quota'), { status: 429, code: 'insufficient_quota' }),
+			],
+			[
+				'nested code',
+				Object.assign(new Error('quota'), { status: 429, error: { code: 'insufficient_quota' } }),
+			],
+		])('should throw when a 429 carries the insufficient_quota code (%s)', (_, error) => {
+			expect(() => n8nDefaultFailedAttemptHandler(error)).toThrow(error);
+			expect(error).not.toHaveProperty('retryAfterMs');
+		});
+
+		it('should throw when a 429 message says the quota is exhausted', () => {
+			const error = Object.assign(new Error('Your credit balance is too low'), { status: 429 });
+			expect(() => n8nDefaultFailedAttemptHandler(error)).toThrow(error);
+			expect(error).not.toHaveProperty('retryAfterMs');
+		});
+
+		it('should read Retry-After from the message when there is no header', () => {
+			const error = Object.assign(new Error('Rate limited. Please try again in 20 seconds.'), {
+				status: 429,
+			});
+			expect(() => n8nDefaultFailedAttemptHandler(error)).not.toThrow();
+			expect(error).toHaveProperty('retryAfterMs', 20000);
+		});
+
+		it('should not throw for a 429 without a Retry-After header', () => {
+			const error = new MockHttpError('Rate limited', 429);
+			expect(() => n8nDefaultFailedAttemptHandler(error)).not.toThrow();
+			expect(error).not.toHaveProperty('retryAfterMs');
+		});
+
+		it('should pass Retry-After to the retry loop', () => {
+			const error = Object.assign(new Error('Rate limited'), {
+				status: 429,
+				headers: new Headers({ 'retry-after': '5' }),
+			});
+			expect(() => n8nDefaultFailedAttemptHandler(error)).not.toThrow();
+			expect(error).toHaveProperty('retryAfterMs', 5000);
+		});
+
+		it('should pass a Retry-After of exactly a minute to the retry loop', () => {
+			const error = Object.assign(new Error('Rate limited'), {
+				status: 429,
+				headers: new Headers({ 'retry-after': '60' }),
+			});
+			expect(() => n8nDefaultFailedAttemptHandler(error)).not.toThrow();
+			expect(error).toHaveProperty('retryAfterMs', 60000);
+		});
+
+		it('should throw when Retry-After is longer than a minute', () => {
+			const error = Object.assign(new Error('Rate limited'), {
+				status: 429,
+				headers: new Headers({ 'retry-after': '120' }),
+			});
+			expect(() => n8nDefaultFailedAttemptHandler(error)).toThrow(error);
+			expect(error).not.toHaveProperty('retryAfterMs');
+		});
+	});
 });

packages/@n8n/ai-utilities/src/__tests__/utils/failed-attempt-handler/n8nLlmFailedAttemptHandler.test.ts

@@ -1,3 +1,4 @@
+import { AsyncCaller } from '@langchain/core/utils/async_caller';
 import type { ISupplyDataFunctions } from 'n8n-workflow';
 import { NodeApiError, UnexpectedError } from 'n8n-workflow';
 import { mock } from 'vitest-mock-extended';
@@ -9,17 +10,17 @@ describe('makeN8nLlmFailedAttemptHandler', () => {
 		getNode: vi.fn(),
 	});
 
-	it('should throw a wrapped error, when NO custom handler is provided', () => {
+	it('should not throw a retryable error, when NO custom handler is provided', () => {
 		const handler = makeN8nLlmFailedAttemptHandler(ctx);
 
-		expect(() => handler(new Error('Test error'))).toThrow(NodeApiError);
+		expect(() => handler(new Error('Test error'))).not.toThrow();
 	});
 
-	it('should wrapped error when custom handler is provided', () => {
+	it('should not throw a retryable error when custom handler is provided', () => {
 		const customHandler = vi.fn();
 		const handler = makeN8nLlmFailedAttemptHandler(ctx, customHandler);
 
-		expect(() => handler(new Error('Test error'))).toThrow(NodeApiError);
+		expect(() => handler(new Error('Test error'))).not.toThrow();
 		expect(customHandler).toHaveBeenCalled();
 	});
 
@@ -33,33 +34,78 @@ describe('makeN8nLlmFailedAttemptHandler', () => {
 		expect(customHandler).toHaveBeenCalled();
 	});
 
-	it('should not throw if retries are left', () => {
-		const customHandler = vi.fn();
-		const handler = makeN8nLlmFailedAttemptHandler(ctx, customHandler);
+	it('should throw NodeApiError when the default handler rejects and the custom one does not', () => {
+		const handler = makeN8nLlmFailedAttemptHandler(ctx, vi.fn());
 
-		const error = new Error('Test error');
-		(error as any).retriesLeft = 1;
-
-		expect(() => handler(error)).not.toThrow();
+		expect(() => handler(Object.assign(new Error('Bad request'), { status: 400 }))).toThrow(
+			NodeApiError,
+		);
 	});
 
-	it('should throw NodeApiError if no retries are left', () => {
-		const handler = makeN8nLlmFailedAttemptHandler(ctx);
+	describe('through AsyncCaller', () => {
+		beforeEach(() => {
+			vi.useFakeTimers();
+		});
 
-		const error = new Error('Test error');
-		(error as any).retriesLeft = 0;
+		afterEach(() => {
+			vi.useRealTimers();
+		});
 
-		expect(() => handler(error)).toThrow(NodeApiError);
-	});
+		const rateLimited = () => Object.assign(new Error('429'), { status: 429 });
 
-	it('should throw NodeApiError if no retries are left with custom handler', () => {
-		const customHandler = vi.fn();
-		const handler = makeN8nLlmFailedAttemptHandler(ctx, customHandler);
+		it('should retry through AsyncCaller up to maxRetries', async () => {
+			const caller = new AsyncCaller({
+				maxRetries: 2,
+				onFailedAttempt: makeN8nLlmFailedAttemptHandler(ctx),
+			});
+			const callable = vi
+				.fn()
+				.mockRejectedValueOnce(rateLimited())
+				.mockRejectedValueOnce(rateLimited())
+				.mockResolvedValueOnce('ok');
 
-		const error = new Error('Test error');
-		(error as any).retriesLeft = 0;
+			const pending = caller.call(callable);
+			await vi.runAllTimersAsync();
 
-		expect(() => handler(error)).toThrow(NodeApiError);
-		expect(customHandler).toHaveBeenCalled();
+			await expect(pending).resolves.toBe('ok');
+			expect(callable).toHaveBeenCalledTimes(3);
+		});
+
+		it('should surface the last error when retries are spent', async () => {
+			const caller = new AsyncCaller({
+				maxRetries: 2,
+				onFailedAttempt: makeN8nLlmFailedAttemptHandler(ctx),
+			});
+			const error = rateLimited();
+			const callable = vi.fn().mockRejectedValue(error);
+
+			const pending = caller.call(callable);
+			// Attach the rejection handler before the timers drain so the rejection is not unhandled
+			const result = expect(pending).rejects.toBe(error);
+			await vi.runAllTimersAsync();
+
+			await result;
+			expect(callable).toHaveBeenCalledTimes(3);
+		});
+
+		it('should honour retryAfterMs in the delay', async () => {
+			const caller = new AsyncCaller({
+				maxRetries: 2,
+				onFailedAttempt: makeN8nLlmFailedAttemptHandler(ctx),
+			});
+			const error = Object.assign(new Error('429'), {
+				status: 429,
+				headers: new Headers({ 'retry-after': '5' }),
+			});
+			const callable = vi.fn().mockRejectedValueOnce(error).mockResolvedValueOnce('ok');
+
+			const pending = caller.call(callable);
+			await vi.advanceTimersByTimeAsync(4999);
+			expect(callable).toHaveBeenCalledTimes(1);
+
+			await vi.advanceTimersByTimeAsync(1);
+			expect(callable).toHaveBeenCalledTimes(2);
+			await expect(pending).resolves.toBe('ok');
+		});
 	});
 });

packages/@n8n/ai-utilities/src/utils/failed-attempt-handler/n8nDefaultFailedAttemptHandler.ts

@@ -1,3 +1,5 @@
+import { classifyRateLimitError } from '@langchain/core/utils/async_caller';
+
 const STATUS_NO_RETRY = [
 	400, // Bad Request
 	401, // Unauthorized
@@ -8,12 +10,16 @@ const STATUS_NO_RETRY = [
 	406, // Not Acceptable
 	407, // Proxy Authentication Required
 	409, // Conflict
+	413, // Payload Too Large
 ];
 
 /**
  * This function is used as a default handler for failed attempts in all LLMs.
  * It is based on a default handler from the langchain core package.
  * It throws an error when it encounters a known error that should not be retried.
+ * A 429 is classified by `classifyRateLimitError`: it is not retried when the code or message
+ * signals an exhausted quota, or when Retry-After is over 60 s. Other 429s are retried, and a
+ * Retry-After within that limit is copied to `retryAfterMs` so the retry loop waits that long.
  * @param error
  */
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -38,4 +44,13 @@ export const n8nDefaultFailedAttemptHandler = (error: any) => {
 	if (status && STATUS_NO_RETRY.includes(+status)) {
 		throw error;
 	}
+
+	const rateLimit = classifyRateLimitError(error);
+	if (rateLimit) {
+		if (rateLimit.action === 'stop') throw error;
+		if (rateLimit.action === 'capacity' && rateLimit.retryAfterMs !== undefined) throw error;
+		if (rateLimit.retryAfterMs !== undefined) {
+			Object.assign(error, { retryAfterMs: rateLimit.retryAfterMs });
+		}
+	}
 };

packages/@n8n/ai-utilities/src/utils/failed-attempt-handler/n8nLlmFailedAttemptHandler.ts

@@ -7,8 +7,8 @@ import { n8nDefaultFailedAttemptHandler } from './n8nDefaultFailedAttemptHandler
 /**
  * This function returns a custom failed attempt handler for using with LangChain models.
  * It first tries to use a custom handler passed as an argument, and if that doesn't throw an error, it uses the default handler.
- * It always wraps the error in a NodeApiError.
- * It throws an error ONLY if there are no retries left.
+ * It throws a NodeApiError only when a handler rejects the error.
+ * Otherwise it returns, and AsyncCaller retries with backoff.
  */
 export const makeN8nLlmFailedAttemptHandler = (
 	ctx: ISupplyDataFunctions,
@@ -29,18 +29,5 @@ export const makeN8nLlmFailedAttemptHandler = (
 
 			throw apiError;
 		}
-
-		// If no error was thrown, check if it is the last retry
-		// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-		if (error?.retriesLeft > 0) {
-			return;
-		}
-
-		// If there are no retries left, throw the error wrapped in a NodeApiError
-		const apiError = new NodeApiError(ctx.getNode(), error as JsonObject, {
-			functionality: 'configuration-node',
-		});
-
-		throw apiError;
 	};
 };

packages/@n8n/nodes-langchain/nodes/embeddings/test/EmbeddingsDatabricks.test.ts

@@ -190,9 +190,12 @@ describe('EmbeddingsDatabricks', () => {
 			await node.supplyData.call(ctx, 0);
 
 			const [, databricksHandler] = mockedMakeN8nLlmFailedAttemptHandler.mock.calls[0];
-			expect(() => databricksHandler?.(Object.assign(new Error('429 x'), { status: 429 }))).toThrow(
-				'Databricks rate limit reached for system.ai.gte-large-en',
-			);
+			const rateLimited = Object.assign(new Error('429 x'), { status: 429 });
+			expect(() => databricksHandler?.(rateLimited)).not.toThrow();
+			expect(rateLimited).toMatchObject({
+				message: 'Databricks rate limit reached for system.ai.gte-large-en',
+				description: expect.stringMatching(/throttling requests to this model service/),
+			});
 		});
 
 		it('should reject non-https hosts', async () => {

packages/@n8n/nodes-langchain/nodes/llms/LmChatDatabricks/test/LmChatDatabricks.test.ts

@@ -174,9 +174,12 @@ describe('LmChatDatabricks', () => {
 			await node.supplyData.call(ctx, 0);
 
 			const [, databricksHandler] = mockedMakeN8nLlmFailedAttemptHandler.mock.calls[0];
-			expect(() => databricksHandler?.(Object.assign(new Error('429 x'), { status: 429 }))).toThrow(
-				'Databricks rate limit reached for my-chat-endpoint',
-			);
+			const rateLimited = Object.assign(new Error('429 x'), { status: 429 });
+			expect(() => databricksHandler?.(rateLimited)).not.toThrow();
+			expect(rateLimited).toMatchObject({
+				message: 'Databricks rate limit reached for my-chat-endpoint',
+				description: expect.stringMatching(/throttling requests to this model service/),
+			});
 		});
 
 		it('should send the bearer and the partner User-Agent on every request', async () => {

packages/@n8n/nodes-langchain/utils/databricks/error-handling.ts

@@ -52,12 +52,16 @@ function isExpiredTokenResponse(error: unknown, expiredStatus: number): boolean
 	);
 }
 
+const RATE_LIMIT_HINT =
+	"Databricks is throttling requests to this model service. Wait and retry, reduce concurrent requests, or ask your workspace admin to raise the endpoint's rate limit.";
+
 /**
  * A dead session arrives either as a rejected response or as a wrapped throw.
  * `expiredStatus` is undefined for grants that cannot be reconnected, such as a
  * service principal — those keep the generic "check your credentials" advice.
- * On a 429 the thrown error names Databricks and `modelName` (the configured model
- * service) and keeps the Databricks text, which carries the endpoint-specific fix.
+ * A 429 is decorated in place so it names Databricks and `modelName` (the
+ * configured model service) and keeps the Databricks text, then returned so the
+ * shared handler retries it with backoff. This handler never throws on 429.
  */
 export function makeDatabricksFailedAttemptHandler(
 	expiredStatus: number | undefined,
@@ -79,10 +83,9 @@ export function makeDatabricksFailedAttemptHandler(
 				isRecord(error.error) && typeof error.error.message === 'string'
 					? `: ${error.error.message}`
 					: '';
-			throw new OperationalError(`Databricks rate limit reached for ${modelName}${detail}`, {
-				description:
-					"Databricks is throttling requests to this model service. Wait and retry, reduce concurrent requests, or ask your workspace admin to raise the endpoint's rate limit.",
-				cause: error,
+			Object.assign(error, {
+				message: `Databricks rate limit reached for ${modelName}${detail}`,
+				description: RATE_LIMIT_HINT,
 			});
 		}
 	};

packages/@n8n/nodes-langchain/utils/databricks/test/error-handling.test.ts

@@ -1,6 +1,9 @@
-import type { INode } from 'n8n-workflow';
-import { OperationalError } from 'n8n-workflow';
+import { AsyncCaller } from '@langchain/core/utils/async_caller';
+import { makeN8nLlmFailedAttemptHandler } from '@n8n/ai-utilities';
+import type { INode, ISupplyDataFunctions } from 'n8n-workflow';
+import { NodeApiError, OperationalError } from 'n8n-workflow';
 import { RateLimitError } from 'openai';
+import { mock } from 'vitest-mock-extended';
 
 import { OAuth2SessionExpiredError } from '../../oauth2-token-provider';
 import { makeDatabricksFailedAttemptHandler, wrapDatabricksErrorFetch } from '../error-handling';
@@ -24,6 +27,18 @@ function apiError(status: number, message: string) {
 	return Object.assign(new Error(message), { status });
 }
 
+const DATABRICKS_TEXT =
+	'REQUEST_LIMIT_EXCEEDED: Exceeded workspace QPS rate limit for databricks-meta-llama-3-3-70b-instruct. Please use a provisioned throughput Foundation Model endpoint';
+
+function databricksRateLimit(headers = new Headers()) {
+	return new RateLimitError(
+		429,
+		{ message: DATABRICKS_TEXT, code: 'REQUEST_LIMIT_EXCEEDED' },
+		undefined,
+		headers,
+	);
+}
+
 describe('makeDatabricksFailedAttemptHandler', () => {
 	it('should ask the user to reconnect when the token was rejected', () => {
 		expect(() => handle(apiError(403, '403 Invalid Token'))).toThrow(OperationalError);
@@ -78,35 +93,92 @@ describe('makeDatabricksFailedAttemptHandler', () => {
 	});
 
 	it('should name Databricks and keep its message when the endpoint is rate limited', () => {
-		const databricksText =
-			'REQUEST_LIMIT_EXCEEDED: Exceeded workspace QPS rate limit for databricks-meta-llama-3-3-70b-instruct. Please use a provisioned throughput Foundation Model endpoint';
-		const rateLimited = new RateLimitError(
-			429,
-			{ message: databricksText, code: 'REQUEST_LIMIT_EXCEEDED' },
-			undefined,
-			new Headers(),
-		);
+		const rateLimited = databricksRateLimit();
 
-		expect(() => handle(rateLimited)).toThrow(OperationalError);
-		expect(() => handle(rateLimited)).toThrow(
-			expect.objectContaining({
-				message: `Databricks rate limit reached for main.default.llama: ${databricksText}`,
-				description: RATE_LIMIT_HINT,
-				cause: rateLimited,
-			}),
-		);
-		expect(() => handle(rateLimited)).not.toThrow(/OpenAI/);
+		expect(() => handle(rateLimited)).not.toThrow();
+		expect(rateLimited).toMatchObject({
+			message: `Databricks rate limit reached for main.default.llama: ${DATABRICKS_TEXT}`,
+			description: RATE_LIMIT_HINT,
+		});
+		expect(rateLimited.message).not.toMatch(/OpenAI/);
+		expect(rateLimited).not.toHaveProperty('retryAfterMs');
 	});
 
 	it('should still explain a rate limit when the response has no body', () => {
 		const rateLimited = apiError(429, '429 status code (no body)');
 
-		expect(() => handle(rateLimited)).toThrow(
-			expect.objectContaining({
-				message: 'Databricks rate limit reached for main.default.llama',
+		expect(() => handle(rateLimited)).not.toThrow();
+		expect(rateLimited).toMatchObject({
+			message: 'Databricks rate limit reached for main.default.llama',
+			description: RATE_LIMIT_HINT,
+		});
+		expect(rateLimited).not.toHaveProperty('retryAfterMs');
+	});
+
+	describe('through the n8n handler chain', () => {
+		const ctx = mock<ISupplyDataFunctions>({ getNode: () => mockNode });
+		const caller = () =>
+			new AsyncCaller({
+				maxRetries: 2,
+				onFailedAttempt: makeN8nLlmFailedAttemptHandler(ctx, handle),
+			});
+
+		beforeEach(() => {
+			vi.useFakeTimers();
+		});
+
+		afterEach(() => {
+			vi.useRealTimers();
+		});
+
+		it('should retry a rate limit and succeed', async () => {
+			const callable = vi
+				.fn()
+				.mockRejectedValueOnce(databricksRateLimit())
+				.mockRejectedValueOnce(databricksRateLimit())
+				.mockResolvedValueOnce('ok');
+
+			const pending = caller().call(callable);
+			await vi.runAllTimersAsync();
+
+			await expect(pending).resolves.toBe('ok');
+			expect(callable).toHaveBeenCalledTimes(3);
+		});
+
+		it('should surface the decorated rate limit error once retries are spent', async () => {
+			const rateLimited = databricksRateLimit();
+			const callable = vi.fn().mockRejectedValue(rateLimited);
+
+			const pending = caller().call(callable);
+			const result = expect(pending).rejects.toBe(rateLimited);
+			await vi.runAllTimersAsync();
+
+			await result;
+			expect(callable).toHaveBeenCalledTimes(3);
+			expect(rateLimited).not.toBeInstanceOf(NodeApiError);
+			expect(rateLimited.message).toBe(
+				`Databricks rate limit reached for main.default.llama: ${DATABRICKS_TEXT}`,
+			);
+			expect(rateLimited).toHaveProperty('description', RATE_LIMIT_HINT);
+		});
+
+		it('should fail at once with the Databricks text when Retry-After is over a minute', async () => {
+			const callable = vi
+				.fn()
+				.mockRejectedValue(databricksRateLimit(new Headers({ 'retry-after': '120' })));
+
+			const pending = caller().call(callable);
+			const result = expect(pending).rejects.toMatchObject({
+				messages: [`Databricks rate limit reached for main.default.llama: ${DATABRICKS_TEXT}`],
 				description: RATE_LIMIT_HINT,
-			}),
-		);
+				httpCode: '429',
+			});
+			await vi.runAllTimersAsync();
+
+			await result;
+			await expect(pending).rejects.toBeInstanceOf(NodeApiError);
+			expect(callable).toHaveBeenCalledTimes(1);
+		});
 	});
 
 	it('should not tell a service principal to sign in again', () => {

packages/@n8n/nodes-langchain/utils/output_parsers/langchainParserError.test.ts

@@ -94,6 +94,16 @@ describe('wrapLangChainParserError', () => {
 			}
 		});
 
+		it("keeps the error's own description instead of the class-name fallback", () => {
+			const original = Object.assign(new Error('rate limited'), { description: 'hint' });
+
+			const wrapped = wrapLangChainParserError(original, node, undefined, {
+				enrichNonParserErrors: true,
+			});
+
+			expect((wrapped as NodeOperationError).description).toBe('hint');
+		});
+
 		it('preserves the underlying class name in the description', () => {
 			const original = new TypeError('not a function');
 

packages/@n8n/nodes-langchain/utils/output_parsers/langchainParserError.ts

@@ -10,8 +10,7 @@ const LANGCHAIN_PARSER_ERROR_MESSAGES = ['Failed to parse. Text:', 'Unable to pa
 
 const AGENT_FAILURE_FALLBACK_MESSAGE = 'Agent execution failed';
 
-function getErrorProperty(error: unknown, property: 'message' | 'name') {
-	if (error instanceof Error) return error[property];
+function getErrorProperty(error: unknown, property: 'message' | 'name' | 'description') {
 	if (typeof error !== 'object' || error === null || !(property in error)) return undefined;
 
 	const value = (error as Record<string, unknown>)[property];
@@ -78,7 +77,9 @@ function wrapAsNodeOperationError(
 
 	const options = {
 		message,
-		description: className ? `Original error: ${className}` : undefined,
+		description:
+			getErrorProperty(error, 'description') ??
+			(className ? `Original error: ${className}` : undefined),
 		level: 'error' as const,
 		...(itemIndex !== undefined ? { itemIndex } : {}),
 	};