Skip to content

Commit 472d5ae

Browse files
Improve session initialization to check storage first (#281)
* Improve session initialization to check storage first Reduces unnecessary "session already exists" errors by checking DO storage before attempting to create a new session. * Add changeset for session initialization fix * 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. * 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. * 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. * 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.
1 parent ae7755c commit 472d5ae

File tree

12 files changed

+82
-32
lines changed

12 files changed

+82
-32
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@cloudflare/sandbox': patch
3+
---
4+
5+
Fix session initialization to eliminate noisy error logs during hot reloads

CLAUDE.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,10 @@ Turbo handles task orchestration (`turbo.json`) with dependency-aware builds.
291291

292292
**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.
293293

294+
**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.
295+
296+
**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.
297+
294298
Good examples:
295299

296300
```

packages/sandbox-container/src/services/session-manager.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,10 @@ export class SessionManager {
3232
success: false,
3333
error: {
3434
message: `Session '${options.id}' already exists`,
35-
code: ErrorCode.INTERNAL_ERROR,
35+
code: ErrorCode.SESSION_ALREADY_EXISTS,
3636
details: {
37-
sessionId: options.id,
38-
originalError: 'Session already exists'
39-
} satisfies InternalErrorContext
37+
sessionId: options.id
38+
}
4039
}
4140
};
4241
}

packages/sandbox/src/errors/adapter.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import type {
2626
PortNotExposedContext,
2727
ProcessErrorContext,
2828
ProcessNotFoundContext,
29+
SessionAlreadyExistsContext,
2930
ValidationFailedContext
3031
} from '@repo/shared/errors';
3132
import { ErrorCode } from '@repo/shared/errors';
@@ -58,6 +59,7 @@ import {
5859
ProcessNotFoundError,
5960
SandboxError,
6061
ServiceNotRespondingError,
62+
SessionAlreadyExistsError,
6163
ValidationFailedError
6264
} from './classes';
6365

@@ -123,6 +125,12 @@ export function createErrorFromResponse(errorResponse: ErrorResponse): Error {
123125
errorResponse as unknown as ErrorResponse<ProcessErrorContext>
124126
);
125127

128+
// Session Errors
129+
case ErrorCode.SESSION_ALREADY_EXISTS:
130+
return new SessionAlreadyExistsError(
131+
errorResponse as unknown as ErrorResponse<SessionAlreadyExistsContext>
132+
);
133+
126134
// Port Errors
127135
case ErrorCode.PORT_ALREADY_EXPOSED:
128136
return new PortAlreadyExposedError(

packages/sandbox/src/errors/classes.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import type {
2828
ProcessExitedBeforeReadyContext,
2929
ProcessNotFoundContext,
3030
ProcessReadyTimeoutContext,
31+
SessionAlreadyExistsContext,
3132
ValidationFailedContext
3233
} from '@repo/shared/errors';
3334

@@ -236,6 +237,25 @@ export class ProcessError extends SandboxError<ProcessErrorContext> {
236237
}
237238
}
238239

240+
// ============================================================================
241+
// Session Errors
242+
// ============================================================================
243+
244+
/**
245+
* Error thrown when a session already exists
246+
*/
247+
export class SessionAlreadyExistsError extends SandboxError<SessionAlreadyExistsContext> {
248+
constructor(errorResponse: ErrorResponse<SessionAlreadyExistsContext>) {
249+
super(errorResponse);
250+
this.name = 'SessionAlreadyExistsError';
251+
}
252+
253+
// Type-safe accessors
254+
get sessionId() {
255+
return this.context.sessionId;
256+
}
257+
}
258+
239259
// ============================================================================
240260
// Port Errors
241261
// ============================================================================

packages/sandbox/src/errors/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ export {
109109
ProcessReadyTimeoutError,
110110
SandboxError,
111111
ServiceNotRespondingError,
112+
// Session Errors
113+
SessionAlreadyExistsError,
112114
// Validation Errors
113115
ValidationFailedError
114116
} from './classes';

packages/sandbox/src/sandbox.ts

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ import {
3737
CustomDomainRequiredError,
3838
ErrorCode,
3939
ProcessExitedBeforeReadyError,
40-
ProcessReadyTimeoutError
40+
ProcessReadyTimeoutError,
41+
SessionAlreadyExistsError
4142
} from './errors';
4243
import { CodeInterpreter } from './interpreter';
4344
import { isLocalhostPattern } from './request-handler';
@@ -994,39 +995,38 @@ export class Sandbox<Env = unknown> extends Container<Env> implements ISandbox {
994995
* we reuse it instead of trying to create a new one.
995996
*/
996997
private async ensureDefaultSession(): Promise<string> {
997-
if (!this.defaultSession) {
998-
const sessionId = `sandbox-${this.sandboxName || 'default'}`;
998+
const sessionId = `sandbox-${this.sandboxName || 'default'}`;
999999

1000-
try {
1001-
// Try to create session in container
1002-
await this.client.utils.createSession({
1003-
id: sessionId,
1004-
env: this.envVars || {},
1005-
cwd: '/workspace'
1006-
});
1000+
// Fast path: session already initialized in this instance
1001+
if (this.defaultSession === sessionId) {
1002+
return this.defaultSession;
1003+
}
1004+
1005+
// Create session in container
1006+
try {
1007+
await this.client.utils.createSession({
1008+
id: sessionId,
1009+
env: this.envVars || {},
1010+
cwd: '/workspace'
1011+
});
10071012

1013+
this.defaultSession = sessionId;
1014+
await this.ctx.storage.put('defaultSession', sessionId);
1015+
this.logger.debug('Default session initialized', { sessionId });
1016+
} catch (error: unknown) {
1017+
// Session may already exist (e.g., after hot reload or concurrent request)
1018+
if (error instanceof SessionAlreadyExistsError) {
1019+
this.logger.debug(
1020+
'Session exists in container but not in DO state, syncing',
1021+
{ sessionId }
1022+
);
10081023
this.defaultSession = sessionId;
1009-
// Persist to storage so it survives hot reloads
10101024
await this.ctx.storage.put('defaultSession', sessionId);
1011-
this.logger.debug('Default session initialized', { sessionId });
1012-
} catch (error: unknown) {
1013-
// If session already exists (e.g., after hot reload), reuse it
1014-
if (
1015-
error instanceof Error &&
1016-
error.message.includes('already exists')
1017-
) {
1018-
this.logger.debug('Reusing existing session after reload', {
1019-
sessionId
1020-
});
1021-
this.defaultSession = sessionId;
1022-
// Persist to storage in case it wasn't saved before
1023-
await this.ctx.storage.put('defaultSession', sessionId);
1024-
} else {
1025-
// Re-throw other errors
1026-
throw error;
1027-
}
1025+
} else {
1026+
throw error;
10281027
}
10291028
}
1029+
10301030
return this.defaultSession;
10311031
}
10321032

packages/shared/src/errors/codes.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ export const ErrorCode = {
4343
PROCESS_PERMISSION_DENIED: 'PROCESS_PERMISSION_DENIED',
4444
PROCESS_ERROR: 'PROCESS_ERROR',
4545

46+
// Session Errors (409)
47+
SESSION_ALREADY_EXISTS: 'SESSION_ALREADY_EXISTS',
48+
4649
// Port Errors (409)
4750
PORT_ALREADY_EXPOSED: 'PORT_ALREADY_EXPOSED',
4851
PORT_IN_USE: 'PORT_IN_USE',

packages/shared/src/errors/contexts.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ export interface ProcessErrorContext {
4848
stderr?: string;
4949
}
5050

51+
export interface SessionAlreadyExistsContext {
52+
sessionId: string;
53+
}
54+
5155
/**
5256
* Process readiness error contexts
5357
*/

packages/shared/src/errors/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export type {
5757
ProcessExitedBeforeReadyContext,
5858
ProcessNotFoundContext,
5959
ProcessReadyTimeoutContext,
60+
SessionAlreadyExistsContext,
6061
ValidationFailedContext
6162
} from './contexts';
6263
// Export utility functions

0 commit comments

Comments
 (0)