From a1748620c080fefbf3bb51424112e75b5467e6fb Mon Sep 17 00:00:00 2001 From: Naresh Date: Sun, 7 Dec 2025 19:16:37 +0000 Subject: [PATCH 1/6] Improve session initialization to check storage first Reduces unnecessary "session already exists" errors by checking DO storage before attempting to create a new session. --- packages/sandbox/src/sandbox.ts | 58 ++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index cc74869c..9ac3e4fd 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -994,39 +994,43 @@ export class Sandbox extends Container implements ISandbox { * we reuse it instead of trying to create a new one. */ private async ensureDefaultSession(): Promise { - if (!this.defaultSession) { - const sessionId = `sandbox-${this.sandboxName || 'default'}`; + const sessionId = `sandbox-${this.sandboxName || 'default'}`; - try { - // Try to create session in container - await this.client.utils.createSession({ - id: sessionId, - env: this.envVars || {}, - cwd: '/workspace' - }); + // Fast path: session already initialized in this instance + if (this.defaultSession === sessionId) { + return this.defaultSession; + } + + // Check storage - session may have been created by a previous call in this request + const storedSession = await this.ctx.storage.get('defaultSession'); + if (storedSession === sessionId) { + this.defaultSession = sessionId; + this.logger.debug('Restored session from storage', { sessionId }); + return this.defaultSession; + } + + // Need to create session + try { + await this.client.utils.createSession({ + id: sessionId, + env: this.envVars || {}, + cwd: '/workspace' + }); + this.defaultSession = sessionId; + await this.ctx.storage.put('defaultSession', sessionId); + this.logger.debug('Default session initialized', { sessionId }); + } catch (error: unknown) { + // If session already exists (container has it but storage didn't), reuse it + if (error instanceof Error && error.message.includes('already exists')) { + this.logger.debug('Reusing existing session', { sessionId }); this.defaultSession = sessionId; - // Persist to storage so it survives hot reloads await this.ctx.storage.put('defaultSession', sessionId); - this.logger.debug('Default session initialized', { sessionId }); - } catch (error: unknown) { - // If session already exists (e.g., after hot reload), reuse it - if ( - error instanceof Error && - error.message.includes('already exists') - ) { - this.logger.debug('Reusing existing session after reload', { - sessionId - }); - this.defaultSession = sessionId; - // Persist to storage in case it wasn't saved before - await this.ctx.storage.put('defaultSession', sessionId); - } else { - // Re-throw other errors - throw error; - } + } else { + throw error; } } + return this.defaultSession; } From 2224500f4288fdaafe650a1d214303e2b9563678 Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 8 Dec 2025 10:39:11 +0000 Subject: [PATCH 2/6] Add changeset for session initialization fix --- .changeset/improve-session-initialization.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/improve-session-initialization.md diff --git a/.changeset/improve-session-initialization.md b/.changeset/improve-session-initialization.md new file mode 100644 index 00000000..af0fbe77 --- /dev/null +++ b/.changeset/improve-session-initialization.md @@ -0,0 +1,5 @@ +--- +'@cloudflare/sandbox': patch +--- + +Improve session initialization to check storage before creating new session From 933cf84a65df310502c8cc8941f5c11a08368bc1 Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 8 Dec 2025 10:39:17 +0000 Subject: [PATCH 3/6] Remove redundant storage check from session initialization The storage check was defensive code that added latency but rarely helped since the constructor already loads defaultSession from storage via blockConcurrencyWhile. --- packages/sandbox/src/sandbox.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 9ac3e4fd..cba10703 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -1001,15 +1001,7 @@ export class Sandbox extends Container implements ISandbox { return this.defaultSession; } - // Check storage - session may have been created by a previous call in this request - const storedSession = await this.ctx.storage.get('defaultSession'); - if (storedSession === sessionId) { - this.defaultSession = sessionId; - this.logger.debug('Restored session from storage', { sessionId }); - return this.defaultSession; - } - - // Need to create session + // Create session in container try { await this.client.utils.createSession({ id: sessionId, @@ -1021,7 +1013,7 @@ export class Sandbox extends Container implements ISandbox { await this.ctx.storage.put('defaultSession', sessionId); this.logger.debug('Default session initialized', { sessionId }); } catch (error: unknown) { - // If session already exists (container has it but storage didn't), reuse it + // Session may already exist (e.g., after hot reload or concurrent request) if (error instanceof Error && error.message.includes('already exists')) { this.logger.debug('Reusing existing session', { sessionId }); this.defaultSession = sessionId; From aab345fadf4ab5ff4ad4916fe6c9e3965194d59c Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 8 Dec 2025 14:03:35 +0000 Subject: [PATCH 4/6] Use RESOURCE_BUSY error code for session conflicts Changes INTERNAL_ERROR (500) to RESOURCE_BUSY (409) for "session already exists" errors. This prevents noisy ERROR-level logs during wrangler dev hot reloads since the SDK only logs 5xx errors. Also improves the debug message to clarify state divergence. --- packages/sandbox-container/src/services/session-manager.ts | 7 +++---- packages/sandbox/src/sandbox.ts | 5 ++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/sandbox-container/src/services/session-manager.ts b/packages/sandbox-container/src/services/session-manager.ts index 7c509dab..0a64ca32 100644 --- a/packages/sandbox-container/src/services/session-manager.ts +++ b/packages/sandbox-container/src/services/session-manager.ts @@ -32,11 +32,10 @@ export class SessionManager { success: false, error: { message: `Session '${options.id}' already exists`, - code: ErrorCode.INTERNAL_ERROR, + code: ErrorCode.RESOURCE_BUSY, details: { - sessionId: options.id, - originalError: 'Session already exists' - } satisfies InternalErrorContext + sessionId: options.id + } } }; } diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index cba10703..ec8dc380 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -1015,7 +1015,10 @@ export class Sandbox extends Container implements ISandbox { } catch (error: unknown) { // Session may already exist (e.g., after hot reload or concurrent request) if (error instanceof Error && error.message.includes('already exists')) { - this.logger.debug('Reusing existing session', { sessionId }); + this.logger.debug( + 'Session exists in container but not in DO state, syncing', + { sessionId } + ); this.defaultSession = sessionId; await this.ctx.storage.put('defaultSession', sessionId); } else { From 282205183f446104904d63c6425d87c417ce167d Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 8 Dec 2025 14:18:23 +0000 Subject: [PATCH 5/6] Use error code instead of string matching for session conflicts String matching on error.message is fragile - if the message wording changes, the code silently breaks. Using error.code is type-safe and reliable. --- .changeset/improve-session-initialization.md | 2 +- CLAUDE.md | 2 ++ packages/sandbox/src/sandbox.ts | 8 ++++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.changeset/improve-session-initialization.md b/.changeset/improve-session-initialization.md index af0fbe77..15092e7d 100644 --- a/.changeset/improve-session-initialization.md +++ b/.changeset/improve-session-initialization.md @@ -2,4 +2,4 @@ '@cloudflare/sandbox': patch --- -Improve session initialization to check storage before creating new session +Fix session initialization to eliminate noisy error logs during hot reloads diff --git a/CLAUDE.md b/CLAUDE.md index e6a66f4b..256c6654 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -291,6 +291,8 @@ Turbo handles task orchestration (`turbo.json`) with dependency-aware builds. **Subject line should stand alone** - don't require reading the body to understand the change. Body is optional and only needed for non-obvious context. +**Focus on the change, not how it was discovered** - never reference "review feedback", "PR comments", or "code review" in commit messages. Describe what the change does and why, not that someone asked for it. + Good examples: ``` diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index ec8dc380..50ccc7e9 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -37,7 +37,8 @@ import { CustomDomainRequiredError, ErrorCode, ProcessExitedBeforeReadyError, - ProcessReadyTimeoutError + ProcessReadyTimeoutError, + SandboxError } from './errors'; import { CodeInterpreter } from './interpreter'; import { isLocalhostPattern } from './request-handler'; @@ -1014,7 +1015,10 @@ export class Sandbox extends Container implements ISandbox { this.logger.debug('Default session initialized', { sessionId }); } catch (error: unknown) { // Session may already exist (e.g., after hot reload or concurrent request) - if (error instanceof Error && error.message.includes('already exists')) { + if ( + error instanceof SandboxError && + error.code === ErrorCode.RESOURCE_BUSY + ) { this.logger.debug( 'Session exists in container but not in DO state, syncing', { sessionId } From 2cbbf09e5e5a140c8b1bf676a46af6f435a0a1d0 Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 8 Dec 2025 14:32:22 +0000 Subject: [PATCH 6/6] Add SESSION_ALREADY_EXISTS error code for session conflicts Replaces RESOURCE_BUSY with a semantically correct error code that properly indicates a uniqueness constraint violation rather than a temporary lock. --- CLAUDE.md | 2 ++ .../src/services/session-manager.ts | 2 +- packages/sandbox/src/errors/adapter.ts | 8 ++++++++ packages/sandbox/src/errors/classes.ts | 20 +++++++++++++++++++ packages/sandbox/src/errors/index.ts | 2 ++ packages/sandbox/src/sandbox.ts | 7 ++----- packages/shared/src/errors/codes.ts | 3 +++ packages/shared/src/errors/contexts.ts | 4 ++++ packages/shared/src/errors/index.ts | 1 + packages/shared/src/errors/status-map.ts | 1 + packages/shared/src/errors/suggestions.ts | 3 +++ 11 files changed, 47 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 256c6654..14e31ebc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -293,6 +293,8 @@ Turbo handles task orchestration (`turbo.json`) with dependency-aware builds. **Focus on the change, not how it was discovered** - never reference "review feedback", "PR comments", or "code review" in commit messages. Describe what the change does and why, not that someone asked for it. +**Avoid bullet points** - write prose, not lists. If you need bullets to explain a change, you're either committing too much at once or over-explaining implementation details. The body should be a brief paragraph, not a changelog. + Good examples: ``` diff --git a/packages/sandbox-container/src/services/session-manager.ts b/packages/sandbox-container/src/services/session-manager.ts index 0a64ca32..a03eb740 100644 --- a/packages/sandbox-container/src/services/session-manager.ts +++ b/packages/sandbox-container/src/services/session-manager.ts @@ -32,7 +32,7 @@ export class SessionManager { success: false, error: { message: `Session '${options.id}' already exists`, - code: ErrorCode.RESOURCE_BUSY, + code: ErrorCode.SESSION_ALREADY_EXISTS, details: { sessionId: options.id } diff --git a/packages/sandbox/src/errors/adapter.ts b/packages/sandbox/src/errors/adapter.ts index 9875da0f..306a46f3 100644 --- a/packages/sandbox/src/errors/adapter.ts +++ b/packages/sandbox/src/errors/adapter.ts @@ -26,6 +26,7 @@ import type { PortNotExposedContext, ProcessErrorContext, ProcessNotFoundContext, + SessionAlreadyExistsContext, ValidationFailedContext } from '@repo/shared/errors'; import { ErrorCode } from '@repo/shared/errors'; @@ -58,6 +59,7 @@ import { ProcessNotFoundError, SandboxError, ServiceNotRespondingError, + SessionAlreadyExistsError, ValidationFailedError } from './classes'; @@ -123,6 +125,12 @@ export function createErrorFromResponse(errorResponse: ErrorResponse): Error { errorResponse as unknown as ErrorResponse ); + // Session Errors + case ErrorCode.SESSION_ALREADY_EXISTS: + return new SessionAlreadyExistsError( + errorResponse as unknown as ErrorResponse + ); + // Port Errors case ErrorCode.PORT_ALREADY_EXPOSED: return new PortAlreadyExposedError( diff --git a/packages/sandbox/src/errors/classes.ts b/packages/sandbox/src/errors/classes.ts index caad715b..ea3c4e11 100644 --- a/packages/sandbox/src/errors/classes.ts +++ b/packages/sandbox/src/errors/classes.ts @@ -28,6 +28,7 @@ import type { ProcessExitedBeforeReadyContext, ProcessNotFoundContext, ProcessReadyTimeoutContext, + SessionAlreadyExistsContext, ValidationFailedContext } from '@repo/shared/errors'; @@ -236,6 +237,25 @@ export class ProcessError extends SandboxError { } } +// ============================================================================ +// Session Errors +// ============================================================================ + +/** + * Error thrown when a session already exists + */ +export class SessionAlreadyExistsError extends SandboxError { + constructor(errorResponse: ErrorResponse) { + super(errorResponse); + this.name = 'SessionAlreadyExistsError'; + } + + // Type-safe accessors + get sessionId() { + return this.context.sessionId; + } +} + // ============================================================================ // Port Errors // ============================================================================ diff --git a/packages/sandbox/src/errors/index.ts b/packages/sandbox/src/errors/index.ts index 98936962..fc3ffa05 100644 --- a/packages/sandbox/src/errors/index.ts +++ b/packages/sandbox/src/errors/index.ts @@ -109,6 +109,8 @@ export { ProcessReadyTimeoutError, SandboxError, ServiceNotRespondingError, + // Session Errors + SessionAlreadyExistsError, // Validation Errors ValidationFailedError } from './classes'; diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 50ccc7e9..ad4b3932 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -38,7 +38,7 @@ import { ErrorCode, ProcessExitedBeforeReadyError, ProcessReadyTimeoutError, - SandboxError + SessionAlreadyExistsError } from './errors'; import { CodeInterpreter } from './interpreter'; import { isLocalhostPattern } from './request-handler'; @@ -1015,10 +1015,7 @@ export class Sandbox extends Container implements ISandbox { this.logger.debug('Default session initialized', { sessionId }); } catch (error: unknown) { // Session may already exist (e.g., after hot reload or concurrent request) - if ( - error instanceof SandboxError && - error.code === ErrorCode.RESOURCE_BUSY - ) { + if (error instanceof SessionAlreadyExistsError) { this.logger.debug( 'Session exists in container but not in DO state, syncing', { sessionId } diff --git a/packages/shared/src/errors/codes.ts b/packages/shared/src/errors/codes.ts index d2b2bba3..7569c5bc 100644 --- a/packages/shared/src/errors/codes.ts +++ b/packages/shared/src/errors/codes.ts @@ -43,6 +43,9 @@ export const ErrorCode = { PROCESS_PERMISSION_DENIED: 'PROCESS_PERMISSION_DENIED', PROCESS_ERROR: 'PROCESS_ERROR', + // Session Errors (409) + SESSION_ALREADY_EXISTS: 'SESSION_ALREADY_EXISTS', + // Port Errors (409) PORT_ALREADY_EXPOSED: 'PORT_ALREADY_EXPOSED', PORT_IN_USE: 'PORT_IN_USE', diff --git a/packages/shared/src/errors/contexts.ts b/packages/shared/src/errors/contexts.ts index 354f6e88..f604e1ed 100644 --- a/packages/shared/src/errors/contexts.ts +++ b/packages/shared/src/errors/contexts.ts @@ -48,6 +48,10 @@ export interface ProcessErrorContext { stderr?: string; } +export interface SessionAlreadyExistsContext { + sessionId: string; +} + /** * Process readiness error contexts */ diff --git a/packages/shared/src/errors/index.ts b/packages/shared/src/errors/index.ts index b294aff8..b1a8bd9a 100644 --- a/packages/shared/src/errors/index.ts +++ b/packages/shared/src/errors/index.ts @@ -57,6 +57,7 @@ export type { ProcessExitedBeforeReadyContext, ProcessNotFoundContext, ProcessReadyTimeoutContext, + SessionAlreadyExistsContext, ValidationFailedContext } from './contexts'; // Export utility functions diff --git a/packages/shared/src/errors/status-map.ts b/packages/shared/src/errors/status-map.ts index d71f1ab6..942172cf 100644 --- a/packages/shared/src/errors/status-map.ts +++ b/packages/shared/src/errors/status-map.ts @@ -42,6 +42,7 @@ export const ERROR_STATUS_MAP: Record = { [ErrorCode.PORT_ALREADY_EXPOSED]: 409, [ErrorCode.PORT_IN_USE]: 409, [ErrorCode.RESOURCE_BUSY]: 409, + [ErrorCode.SESSION_ALREADY_EXISTS]: 409, // 502 Bad Gateway [ErrorCode.SERVICE_NOT_RESPONDING]: 502, diff --git a/packages/shared/src/errors/suggestions.ts b/packages/shared/src/errors/suggestions.ts index fabd5aa7..a3b484b4 100644 --- a/packages/shared/src/errors/suggestions.ts +++ b/packages/shared/src/errors/suggestions.ts @@ -30,6 +30,9 @@ export function getSuggestion( case ErrorCode.PORT_IN_USE: return `Port ${context.port} is already in use by another service. Choose a different port`; + case ErrorCode.SESSION_ALREADY_EXISTS: + return `Session "${context.sessionId}" already exists. Use a different session ID or reuse the existing session`; + case ErrorCode.INVALID_PORT: return `Port must be between 1 and 65535. Port ${context.port} is ${context.reason}`;