From 80ea25b76ea141e0a24f9a17fbd6049aab32af8a Mon Sep 17 00:00:00 2001 From: Albert Alises Date: Thu, 16 Jul 2026 16:13:48 +0200 Subject: [PATCH] fix(core): Retry transient sandbox file writes (#34319) --- .../workspace/__tests__/sandbox-fs.test.ts | 38 +++++ .../workspace/__tests__/sandbox-setup.test.ts | 4 + .../__tests__/workspace-files.test.ts | 140 ++++++++++++++++++ .../instance-ai/src/workspace/sandbox-fs.ts | 126 +++++++++++----- .../src/workspace/workspace-files.ts | 48 +++++- 5 files changed, 315 insertions(+), 41 deletions(-) diff --git a/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-fs.test.ts b/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-fs.test.ts index dd17748ad3f..ee59b3d6dbb 100644 --- a/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-fs.test.ts +++ b/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-fs.test.ts @@ -7,6 +7,10 @@ import { runInSandbox, } from '../sandbox-fs'; +class TransientWriteError extends Error { + statusCode = 524; +} + function createMockWorkspace(overrides?: { executeCommand?: Mock; processes?: { spawn: Mock }; @@ -135,6 +139,40 @@ describe('writeFileViaSandbox', () => { await expect(writeFileViaSandbox(workspace, '/home/user/test.ts', 'content')).rejects.toThrow( 'Failed to write file /home/user/test.ts: permission denied', ); + expect(executeCommand).toHaveBeenCalledTimes(3); + }); + + it('retries transient provider errors while writing', async () => { + const logger = { warn: vi.fn() }; + const executeCommand = vi + .fn() + .mockRejectedValueOnce(new TransientWriteError('gateway timeout')) + .mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); + const workspace = createMockWorkspace({ executeCommand }); + + await writeFileViaSandbox(workspace, 'test.ts', 'hello', { + logger, + retryBackoffBaseMs: 1, + }); + + expect(executeCommand).toHaveBeenCalledTimes(4); + expect(logger.warn).toHaveBeenCalledWith( + 'Sandbox file I/O hit a transient error; retrying', + expect.objectContaining({ path: 'test.ts', attempt: 1 }), + ); + }); + + it('retries transient provider errors while reading and keeps missing-file semantics', async () => { + const executeCommand = vi + .fn() + .mockRejectedValueOnce(new TransientWriteError('gateway timeout')) + .mockResolvedValue({ exitCode: 1, stdout: '', stderr: '' }); + const workspace = createMockWorkspace({ executeCommand }); + + await expect( + readFileViaSandbox(workspace, 'missing.ts', { retryBackoffBaseMs: 1 }), + ).resolves.toBeNull(); + expect(executeCommand).toHaveBeenCalledTimes(2); }); it('should skip mkdir when file has no parent directory', async () => { diff --git a/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-setup.test.ts b/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-setup.test.ts index cb5368713f4..40b3c689452 100644 --- a/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-setup.test.ts +++ b/packages/@n8n/instance-ai/src/workspace/__tests__/sandbox-setup.test.ts @@ -55,6 +55,8 @@ vi.mock('../sandbox-fs', () => ({ throw new Error(`Failed to write file ${path}: ${result.stderr}`); } }, + retryTransientSandboxIo: async (op: () => Promise) => await op(), + isTransientSandboxIoError: () => false, escapeSingleQuotes: (value: string) => value.replace(/'/g, "'\\''"), })); @@ -201,6 +203,8 @@ async function loadLinkWorkspaceSdkWithMocks( runInSandbox, readFileViaSandbox: vi.fn(), writeFileViaSandbox: vi.fn(), + retryTransientSandboxIo: async (op: () => Promise) => await op(), + isTransientSandboxIoError: () => false, escapeSingleQuotes: (value: string) => value.replace(/'/g, "'\\''"), })); diff --git a/packages/@n8n/instance-ai/src/workspace/__tests__/workspace-files.test.ts b/packages/@n8n/instance-ai/src/workspace/__tests__/workspace-files.test.ts index 51bcebf2c4d..6e18719bfe9 100644 --- a/packages/@n8n/instance-ai/src/workspace/__tests__/workspace-files.test.ts +++ b/packages/@n8n/instance-ai/src/workspace/__tests__/workspace-files.test.ts @@ -1,6 +1,14 @@ import type { WorkspaceFileTarget } from '../workspace-files'; import { readWorkspaceFile, writeWorkspaceFile, writeWorkspaceFileMap } from '../workspace-files'; +class TransientWriteError extends Error { + statusCode = 524; +} + +function createLogger() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; +} + function createWorkspaceTarget(files: Map): { target: WorkspaceFileTarget; writes: Map; @@ -78,6 +86,66 @@ describe('workspace-files', () => { await expect(readWorkspaceFile(target, '/tmp/manifest.json')).resolves.toBe('{"ok":true}'); }); + it('retries transient filesystem read errors instead of reporting a missing file', async () => { + const readFile = vi + .fn() + .mockRejectedValueOnce(new TransientWriteError('gateway timeout')) + .mockResolvedValue('{"ok":true}'); + const target: WorkspaceFileTarget = { + filesystem: { readFile, writeFile: vi.fn(async () => {}) }, + }; + + await expect( + readWorkspaceFile(target, '/tmp/manifest.json', { + logger: createLogger(), + retryBackoffBaseMs: 1, + }), + ).resolves.toBe('{"ok":true}'); + expect(readFile).toHaveBeenCalledTimes(2); + }); + + it('throws on exhausted transient read errors instead of returning null', async () => { + const readFile = vi.fn(async () => await Promise.reject(new TransientWriteError('timeout'))); + const target: WorkspaceFileTarget = { + filesystem: { readFile, writeFile: vi.fn(async () => {}) }, + }; + + await expect( + readWorkspaceFile(target, '/tmp/manifest.json', { + logger: createLogger(), + retryBackoffBaseMs: 1, + }), + ).rejects.toThrow('Failed to read workspace file "/tmp/manifest.json"'); + expect(readFile).toHaveBeenCalledTimes(3); + }); + + it('still returns null for non-transient read misses', async () => { + const readFile = vi.fn(async () => await Promise.reject(new Error('ENOENT'))); + const target: WorkspaceFileTarget = { + filesystem: { readFile, writeFile: vi.fn(async () => {}) }, + }; + + await expect( + readWorkspaceFile(target, '/tmp/manifest.json', { logger: createLogger() }), + ).resolves.toBeNull(); + expect(readFile).toHaveBeenCalledTimes(1); + }); + + it('throws on exhausted transient command read errors', async () => { + const executeCommand = vi.fn(async () => { + return await Promise.reject(new TransientWriteError('bad gateway')); + }); + const target: WorkspaceFileTarget = { sandbox: { executeCommand } }; + + await expect( + readWorkspaceFile(target, '/tmp/manifest.json', { + logger: createLogger(), + retryBackoffBaseMs: 1, + }), + ).rejects.toThrow('Failed to read workspace file "/tmp/manifest.json"'); + expect(executeCommand).toHaveBeenCalledTimes(3); + }); + it('writes via filesystem and supports batch writes', async () => { const { target, writes } = createWorkspaceTarget(new Map()); @@ -95,6 +163,78 @@ describe('workspace-files', () => { expect(writes.get('/tmp/c.txt')).toBe('gamma'); }); + it('retries filesystem writes on transient upstream errors', async () => { + const { target, writes } = createWorkspaceTarget(new Map()); + const writeFile = vi + .fn() + .mockRejectedValueOnce(new TransientWriteError('gateway timeout')) + .mockRejectedValueOnce(new TransientWriteError('gateway timeout')) + .mockImplementation(async (path: string, content: string) => { + writes.set(path, content); + await Promise.resolve(); + }); + target.filesystem = { writeFile }; + + await writeWorkspaceFile(target, '/tmp/a.txt', 'alpha', { + logger: createLogger(), + retryBackoffBaseMs: 1, + }); + + expect(writeFile).toHaveBeenCalledTimes(3); + expect(writes.get('/tmp/a.txt')).toBe('alpha'); + expect(target.sandbox?.executeCommand).not.toHaveBeenCalled(); + }); + + it('does not retry non-transient filesystem errors before falling back', async () => { + const { target, writes } = createWorkspaceTarget(new Map()); + const writeFile = vi.fn(async () => await Promise.reject(new Error('permission denied'))); + target.filesystem = { writeFile }; + + await writeWorkspaceFile(target, '/tmp/a.txt', 'alpha', { + logger: createLogger(), + retryBackoffBaseMs: 1, + }); + + expect(writeFile).toHaveBeenCalledTimes(1); + expect(writes.size).toBe(0); + expect(target.sandbox?.executeCommand).toHaveBeenCalled(); + }); + + it('retries sandbox command writes on transient upstream errors', async () => { + let calls = 0; + const executeCommand = vi.fn(async (command: string) => { + calls++; + if (calls === 1) throw new TransientWriteError('bad gateway'); + return await Promise.resolve({ exitCode: 0, stdout: '', stderr: '', command }); + }); + const target: WorkspaceFileTarget = { sandbox: { executeCommand } }; + + await expect( + writeWorkspaceFile(target, '/tmp/a.txt', 'alpha', { + logger: createLogger(), + retryBackoffBaseMs: 1, + }), + ).resolves.toBeUndefined(); + }); + + it('surfaces the error after exhausting transient-write retries on both paths', async () => { + const writeFile = vi.fn(async () => await Promise.reject(new TransientWriteError('timeout'))); + const executeCommand = vi.fn( + async () => await Promise.reject(new TransientWriteError('timeout')), + ); + const target: WorkspaceFileTarget = { filesystem: { writeFile }, sandbox: { executeCommand } }; + + await expect( + writeWorkspaceFile(target, '/tmp/a.txt', 'alpha', { + logger: createLogger(), + retryBackoffBaseMs: 1, + }), + ).rejects.toThrow('command fallback failed'); + + expect(writeFile).toHaveBeenCalledTimes(3); + expect(executeCommand).toHaveBeenCalledTimes(3); + }); + it('logs successful command fallback at warn level', async () => { const { target } = createWorkspaceTarget(new Map()); const writeError = new Error('filesystem unavailable'); diff --git a/packages/@n8n/instance-ai/src/workspace/sandbox-fs.ts b/packages/@n8n/instance-ai/src/workspace/sandbox-fs.ts index 27472f0ecaa..a8d97bcfdba 100644 --- a/packages/@n8n/instance-ai/src/workspace/sandbox-fs.ts +++ b/packages/@n8n/instance-ai/src/workspace/sandbox-fs.ts @@ -14,6 +14,10 @@ import { type SandboxWorkspace as SharedSandboxWorkspace, } from '@n8n/agents/sandbox'; +import { formatErrorForLog } from '../error-formatting'; +import type { Logger } from '../logger'; +import { getTemplateTelemetrySession } from './template-telemetry'; + export interface SandboxWorkspace extends SharedSandboxWorkspace { filesystem?: { provider?: string; @@ -29,9 +33,52 @@ export interface SandboxWorkspace extends SharedSandboxWorkspace { } & NonNullable; } -import { getTemplateTelemetrySession } from './template-telemetry'; - const BASE64_WRITE_CHUNK_SIZE = 32_000; +const SANDBOX_IO_MAX_ATTEMPTS = 3; +const DEFAULT_SANDBOX_IO_RETRY_BACKOFF_BASE_MS = 1_000; +const SANDBOX_IO_RETRY_BACKOFF_CAP_MS = 5_000; + +export interface SandboxIoRetryOptions { + logger?: Pick; + resourceLabel?: string; + retryBackoffBaseMs?: number; +} + +function ioResourceLabel(options?: SandboxIoRetryOptions): string { + return options?.resourceLabel ?? 'Sandbox file'; +} + +async function sleep(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + +// Daytona surfaces upstream gateway failures (e.g. Cloudflare 502/524) as errors with a numeric status. +export function isTransientSandboxIoError(error: unknown): boolean { + if (typeof error !== 'object' || error === null) return false; + const status = 'statusCode' in error ? error.statusCode : 'status' in error ? error.status : null; + return typeof status === 'number' && (status >= 500 || status === 408 || status === 429); +} + +export async function retryTransientSandboxIo( + op: () => Promise, + filePath: string, + options?: SandboxIoRetryOptions, +): Promise { + const baseMs = options?.retryBackoffBaseMs ?? DEFAULT_SANDBOX_IO_RETRY_BACKOFF_BASE_MS; + for (let attempt = 1; ; attempt++) { + try { + return await op(); + } catch (error) { + if (attempt >= SANDBOX_IO_MAX_ATTEMPTS || !isTransientSandboxIoError(error)) throw error; + options?.logger?.warn(`${ioResourceLabel(options)} I/O hit a transient error; retrying`, { + path: filePath, + attempt, + error: formatErrorForLog(error), + }); + await sleep(Math.min(baseMs * 2 ** (attempt - 1), SANDBOX_IO_RETRY_BACKOFF_CAP_MS)); + } + } +} /** * Execute a shell command in the sandbox and wait for completion. @@ -70,53 +117,66 @@ export async function writeFileViaSandbox( workspace: SandboxCommandTarget, filePath: string, content: string | Buffer, + options?: SandboxIoRetryOptions, ): Promise { - const runWriteCommand = async (command: string) => { - const result = await runInSandbox(workspace, command); - if (result.exitCode !== 0) { - throw new Error(`Failed to write file ${filePath}: ${result.stderr}`); - } - }; + await retryTransientSandboxIo( + async () => { + const runWriteCommand = async (command: string) => { + const result = await runInSandbox(workspace, command); + if (result.exitCode !== 0) { + throw new Error(`Failed to write file ${filePath}: ${result.stderr}`); + } + }; - // Ensure parent directory exists - const dir = filePath.substring(0, filePath.lastIndexOf('/')); - if (dir) { - await runWriteCommand(`mkdir -p '${escapeSingleQuotes(dir)}'`); - } + // Ensure parent directory exists + const dir = filePath.substring(0, filePath.lastIndexOf('/')); + if (dir) { + await runWriteCommand(`mkdir -p '${escapeSingleQuotes(dir)}'`); + } - // Encode content as base64, transfer it in small chunks, then decode in the sandbox. - // Some providers run commands through spawn(), where a single huge argument can hit E2BIG. - const b64 = - typeof content === 'string' - ? Buffer.from(content, 'utf-8').toString('base64') - : content.toString('base64'); - const tempPath = `${filePath}.base64.tmp-${Date.now()}-${Math.random().toString(36).slice(2)}`; - const escapedTempPath = escapeSingleQuotes(tempPath); + // Encode content as base64, transfer it in small chunks, then decode in the sandbox. + // Some providers run commands through spawn(), where a single huge argument can hit E2BIG. + const b64 = + typeof content === 'string' + ? Buffer.from(content, 'utf-8').toString('base64') + : content.toString('base64'); + const tempPath = `${filePath}.base64.tmp-${Date.now()}-${Math.random().toString(36).slice(2)}`; + const escapedTempPath = escapeSingleQuotes(tempPath); - await runWriteCommand(`: > '${escapedTempPath}'`); + await runWriteCommand(`: > '${escapedTempPath}'`); - for (let offset = 0; offset < b64.length; offset += BASE64_WRITE_CHUNK_SIZE) { - const chunk = b64.slice(offset, offset + BASE64_WRITE_CHUNK_SIZE); - await runWriteCommand(`printf '%s' '${chunk}' >> '${escapedTempPath}'`); - } + for (let offset = 0; offset < b64.length; offset += BASE64_WRITE_CHUNK_SIZE) { + const chunk = b64.slice(offset, offset + BASE64_WRITE_CHUNK_SIZE); + await runWriteCommand(`printf '%s' '${chunk}' >> '${escapedTempPath}'`); + } - // Decode + cleanup in one shell expression; the exit reflects base64's - // status. Avoid the variable name `status` — it's a read-only builtin in - // zsh, which silently breaks the assignment and loses base64's exit code. - await runWriteCommand( - `base64 -d '${escapedTempPath}' > '${escapeSingleQuotes(filePath)}'; rc=$?; rm -f '${escapedTempPath}'; exit $rc`, + // Decode + cleanup in one shell expression; the exit reflects base64's + // status. Avoid the variable name `status` — it's a read-only builtin in + // zsh, which silently breaks the assignment and loses base64's exit code. + await runWriteCommand( + `base64 -d '${escapedTempPath}' > '${escapeSingleQuotes(filePath)}'; rc=$?; rm -f '${escapedTempPath}'; exit $rc`, + ); + }, + filePath, + options, ); } /** * Read a file from the sandbox via shell command. - * Returns null if the file doesn't exist. + * Returns null if the file doesn't exist. Transient provider errors are + * retried and, when exhausted, thrown — they are not a missing file. */ export async function readFileViaSandbox( workspace: SandboxCommandTarget, filePath: string, + options?: SandboxIoRetryOptions, ): Promise { - const result = await runInSandbox(workspace, `cat '${escapeSingleQuotes(filePath)}' 2>/dev/null`); + const result = await retryTransientSandboxIo( + async () => await runInSandbox(workspace, `cat '${escapeSingleQuotes(filePath)}' 2>/dev/null`), + filePath, + options, + ); if (result.exitCode !== 0) return null; return result.stdout; } diff --git a/packages/@n8n/instance-ai/src/workspace/workspace-files.ts b/packages/@n8n/instance-ai/src/workspace/workspace-files.ts index c303c13403d..a3dcb2f2941 100644 --- a/packages/@n8n/instance-ai/src/workspace/workspace-files.ts +++ b/packages/@n8n/instance-ai/src/workspace/workspace-files.ts @@ -1,6 +1,12 @@ import { formatErrorForLog } from '../error-formatting'; import type { Logger } from '../logger'; -import { readFileViaSandbox, writeFileViaSandbox, type SandboxWorkspace } from './sandbox-fs'; +import { + isTransientSandboxIoError, + readFileViaSandbox, + retryTransientSandboxIo, + writeFileViaSandbox, + type SandboxWorkspace, +} from './sandbox-fs'; export interface WorkspaceFileTarget { filesystem?: { @@ -18,6 +24,8 @@ export interface WorkspaceFileOptions { logger: Logger; /** Used in log and error messages, e.g. "Knowledge base file". */ resourceLabel?: string; + /** Base for the exponential retry backoff on transient write errors. Default 1s. */ + retryBackoffBaseMs?: number; } function resourceLabel(options?: WorkspaceFileOptions): string { @@ -38,10 +46,24 @@ export async function readWorkspaceFile( options?: WorkspaceFileOptions, ): Promise { const filesystem = workspace.filesystem; - if (filesystem?.readFile) { + const readFile = filesystem?.readFile; + if (filesystem && readFile) { try { - return decodeWorkspaceFileContent(await filesystem.readFile(filePath, { encoding: 'utf-8' })); + return decodeWorkspaceFileContent( + await retryTransientSandboxIo( + // .call preserves the provider's `this` binding (e.g. LazyRuntimeFilesystem). + async () => await readFile.call(filesystem, filePath, { encoding: 'utf-8' }), + filePath, + options, + ), + ); } catch (error) { + // A provider failure is not a missing file — surface it instead of reporting null. + if (isTransientSandboxIoError(error)) { + throw new Error( + `Failed to read ${resourceLabel(options).toLowerCase()} "${filePath}": ${formatErrorForLog(error)}`, + ); + } options?.logger.debug(`${resourceLabel(options)} filesystem read missed`, { path: filePath, error: formatErrorForLog(error), @@ -53,8 +75,13 @@ export async function readWorkspaceFile( if (!workspace.sandbox) return null; try { - return await readFileViaSandbox(workspace, filePath); + return await readFileViaSandbox(workspace, filePath, options); } catch (error) { + if (isTransientSandboxIoError(error)) { + throw new Error( + `Failed to read ${resourceLabel(options).toLowerCase()} "${filePath}": ${formatErrorForLog(error)}`, + ); + } options?.logger.debug(`${resourceLabel(options)} command read missed`, { path: filePath, error: formatErrorForLog(error), @@ -75,13 +102,18 @@ export async function writeWorkspaceFile( ): Promise { const label = resourceLabel(options); - if (workspace.filesystem) { + const filesystem = workspace.filesystem; + if (filesystem) { try { - await workspace.filesystem.writeFile(filePath, content, { recursive: true }); + await retryTransientSandboxIo( + async () => await filesystem.writeFile(filePath, content, { recursive: true }), + filePath, + options, + ); return; } catch (error) { try { - await writeFileViaSandbox(workspace, filePath, content); + await writeFileViaSandbox(workspace, filePath, content, options); options?.logger.warn(`${label} filesystem write failed; used command fallback`, { path: filePath, error: formatErrorForLog(error), @@ -96,7 +128,7 @@ export async function writeWorkspaceFile( } try { - await writeFileViaSandbox(workspace, filePath, content); + await writeFileViaSandbox(workspace, filePath, content, options); } catch (error) { throw new Error( `Failed to write ${label.toLowerCase()} "${filePath}": ${formatErrorForLog(error)}`,