diff --git a/.changeset/improve-session-initialization.md b/.changeset/improve-session-initialization.md new file mode 100644 index 00000000..15092e7d --- /dev/null +++ b/.changeset/improve-session-initialization.md @@ -0,0 +1,5 @@ +--- +'@cloudflare/sandbox': patch +--- + +Fix session initialization to eliminate noisy error logs during hot reloads diff --git a/CLAUDE.md b/CLAUDE.md index e6a66f4b..14e31ebc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -291,6 +291,10 @@ 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. + +**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 7c509dab..a03eb740 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.SESSION_ALREADY_EXISTS, details: { - sessionId: options.id, - originalError: 'Session already exists' - } satisfies InternalErrorContext + 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 cc74869c..ad4b3932 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -37,7 +37,8 @@ import { CustomDomainRequiredError, ErrorCode, ProcessExitedBeforeReadyError, - ProcessReadyTimeoutError + ProcessReadyTimeoutError, + SessionAlreadyExistsError } from './errors'; import { CodeInterpreter } from './interpreter'; import { isLocalhostPattern } from './request-handler'; @@ -994,39 +995,38 @@ 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; + } + + // Create session in container + 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) { + // Session may already exist (e.g., after hot reload or concurrent request) + if (error instanceof SessionAlreadyExistsError) { + this.logger.debug( + 'Session exists in container but not in DO state, syncing', + { 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; } 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}`;