From dd0464632b6ab5aa4e50c847cfc0ed336e389c36 Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 19 Nov 2025 16:13:32 +0000 Subject: [PATCH 1/3] Remove unused AsyncLocalStorage infrastructure The AsyncLocalStorage infrastructure (loggerStorage, getLogger, runWithLogger) was set up but never actually used. The codebase uses explicit logger passing via constructor injection throughout. This change removes the unused code, simplifies the logging module, and documents the explicit passing pattern as the canonical approach. The pattern works well for all scenarios including request context, background operations, lifecycle events, and long-lived objects. No breaking changes as the removed functions were never called. --- CLAUDE.md | 19 ++++- package-lock.json | 37 ++++------ packages/sandbox-container/src/config.ts | 19 ----- packages/sandbox/src/openai/index.ts | 78 +++++++++++--------- packages/sandbox/src/sandbox.ts | 71 +++++++++--------- packages/shared/src/index.ts | 2 - packages/shared/src/logger/index.ts | 93 ++---------------------- packages/shared/tests/logger.test.ts | 83 +-------------------- 8 files changed, 115 insertions(+), 287 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b048d593..f2381a32 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -331,9 +331,22 @@ shared package for consistency across the SDK. ### Logging -- Centralized logger from `@repo/shared` -- Structured logging with component context -- Configurable via `SANDBOX_LOG_LEVEL` and `SANDBOX_LOG_FORMAT` env vars +**Pattern**: Explicit logger passing via constructor injection throughout the codebase. + +```typescript +class MyService { + constructor(private logger: Logger) {} + + async doWork() { + const childLogger = this.logger.child({ operation: 'work' }); + childLogger.info('Working', { context }); + } +} +``` + +**Configuration**: `SANDBOX_LOG_LEVEL` (debug|info|warn|error) and `SANDBOX_LOG_FORMAT` (json|pretty) env vars, read at startup. + +**Testing**: Use `createNoOpLogger()` from `@repo/shared` in tests. ### Session Management diff --git a/package-lock.json b/package-lock.json index 62d521cc..e5741407 100644 --- a/package-lock.json +++ b/package-lock.json @@ -218,7 +218,6 @@ "integrity": "sha512-e7jT4DxYvIDLk1ZHmU/m/mB19rex9sv0c2ftBtjSBv+kVM/902eh0fINUzD7UwLLNR+jU585GxUJ8/EBfAM5fw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.27.1", "@babel/generator": "^7.28.5", @@ -1716,7 +1715,6 @@ "integrity": "sha512-Wj7/AMtE9MRnAXa6Su3Lk0LNCfqDYgfwVjwRFVum9U7wsto1imuHqk4kTm7Jni+5A0Hn7dttL6O/zjvUvoo+8A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "defu": "^6.1.4", "exsolve": "^1.0.7", @@ -1850,8 +1848,7 @@ "resolved": "https://registry.npmjs.org/@cloudflare/workers-types/-/workers-types-4.20251115.0.tgz", "integrity": "sha512-aM7jp7IfKhqKvfSaK1IhVTbSzxB6KQ4gX8e/W29tOuZk+YHlYXuRd/bMm4hWkfd7B1HWNWdsx1GTaEUoZIuVsw==", "dev": true, - "license": "MIT OR Apache-2.0", - "peer": true + "license": "MIT OR Apache-2.0" }, "node_modules/@cspotcode/source-map-support": { "version": "0.8.1", @@ -3062,7 +3059,6 @@ "integrity": "sha512-/g2d4sW9nUDJOMz3mabVQvOGhVa4e/BN/Um7yca9Bb2XTzPPnfTWHWQg+IsEYO7M3Vx+EXvaM/I2pJWIMun1bg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@octokit/auth-token": "^4.0.0", "@octokit/graphql": "^7.1.0", @@ -4425,7 +4421,6 @@ "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.5.tgz", "integrity": "sha512-keKxkZMqnDicuvFoJbzrhbtdLSPhj/rZThDlKWCDbgXmUg0rEUFtRssDXKYmtXluZlIqiC5VqkCgRwzuyLHKHw==", "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -4562,7 +4557,6 @@ "integrity": "sha512-oukfKT9Mk41LreEW09vt45f8wx7DordoWUZMYdY/cyAk7w5TWkTRCNZYF7sX7n2wB7jyGAl74OxgwhPgKaqDMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "pathe": "^2.0.3", @@ -4578,7 +4572,6 @@ "integrity": "sha512-dEYtS7qQP2CjU27QBC5oUOxLE/v5eLkGqPE0ZKEIDGMs4vKWe7IjgLOeauHsR0D5YuuycGRO5oSRXnwnmA78fQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/pretty-format": "3.2.4", "magic-string": "^0.30.17", @@ -4607,7 +4600,6 @@ "integrity": "sha512-hGISOaP18plkzbWEcP/QvtRW1xDXF2+96HbEX6byqQhAUbiS5oH6/9JwW+QsQCIYON2bI6QZBF+2PvOmrRZ9wA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "fflate": "^0.8.2", @@ -4912,7 +4904,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.19", "caniuse-lite": "^1.0.30001751", @@ -6840,6 +6831,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -6861,6 +6853,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -6882,6 +6875,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -6903,6 +6897,7 @@ "os": [ "freebsd" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -6924,6 +6919,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -6945,6 +6941,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -6966,6 +6963,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -6987,6 +6985,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -7008,6 +7007,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -7029,6 +7029,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -7050,6 +7051,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -7093,6 +7095,7 @@ "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz", "integrity": "sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==", "license": "MIT", + "peer": true, "dependencies": { "js-tokens": "^3.0.0 || ^4.0.0" }, @@ -8790,7 +8793,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.0.tgz", "integrity": "sha512-tmbWg6W31tQLeB5cdIBOicJDJRR2KzXsV7uSK9iNfLWQ5bIZfxuPEHp7M8wiHyHnn0DD1i7w3Zmin0FtkrwoCQ==", "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -8811,7 +8813,8 @@ "version": "16.13.1", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz", "integrity": "sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/react-katex": { "version": "3.1.0", @@ -9006,7 +9009,6 @@ "integrity": "sha512-JFULvCNl/anKn99eKjOSEubi0lLmNqQDAjyEMME2T4CwezUDL0i6t1O9xZsu2OMehPnV2caNefWpGF+8TnzB6A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@oxc-project/types": "=0.97.0", "@rolldown/pluginutils": "1.0.0-beta.50" @@ -9885,7 +9887,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -10068,7 +10069,6 @@ "integrity": "sha512-ytQKuwgmrrkDTFP4LjR0ToE2nqgy886GpvRSpU0JAnrdBYppuY5rLkRUYPU1yCryb24SsKBTL/hlDQAEFVwtZg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -10277,7 +10277,6 @@ "integrity": "sha512-i7qRCmY42zmCwnYlh9H2SvLEypEFGye5iRmEMKjcGi7zk9UquigRjFtTLz0TYqr0ZGLZhaMHl/foy1bZR+Cwlw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "pathe": "^2.0.3" } @@ -10519,7 +10518,6 @@ "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -10636,7 +10634,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -10650,7 +10647,6 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -10806,7 +10802,6 @@ "dev": true, "hasInstallScript": true, "license": "Apache-2.0", - "peer": true, "bin": { "workerd": "bin/workerd" }, @@ -11457,7 +11452,6 @@ "dev": true, "hasInstallScript": true, "license": "Apache-2.0", - "peer": true, "bin": { "workerd": "bin/workerd" }, @@ -11569,7 +11563,6 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/packages/sandbox-container/src/config.ts b/packages/sandbox-container/src/config.ts index 9a99fe26..7bc7eef1 100644 --- a/packages/sandbox-container/src/config.ts +++ b/packages/sandbox-container/src/config.ts @@ -1,20 +1,3 @@ -/** - * Log level for the sandbox container. - * - * Default: info - * Environment variable: SANDBOX_LOG_LEVEL - */ -const SANDBOX_LOG_LEVEL = process.env.SANDBOX_LOG_LEVEL || 'info'; - -/** - * Log format for the sandbox container. - * - * Default: json - * Set to "pretty" to enable colored, human-readable output. - * Environment variable: SANDBOX_LOG_FORMAT - */ -const SANDBOX_LOG_FORMAT = process.env.SANDBOX_LOG_FORMAT || 'json'; - /** * How long to wait for an interpreter process to spawn and become ready. * If an interpreter doesn't start within this time, something is fundamentally @@ -82,8 +65,6 @@ const STREAM_CHUNK_DELAY_MS = 100; const DEFAULT_CWD = '/workspace'; export const CONFIG = { - SANDBOX_LOG_LEVEL, - SANDBOX_LOG_FORMAT, INTERPRETER_SPAWN_TIMEOUT_MS, INTERPRETER_EXECUTION_TIMEOUT_MS, COMMAND_TIMEOUT_MS, diff --git a/packages/sandbox/src/openai/index.ts b/packages/sandbox/src/openai/index.ts index a2b5311b..a3588813 100644 --- a/packages/sandbox/src/openai/index.ts +++ b/packages/sandbox/src/openai/index.ts @@ -35,11 +35,6 @@ export interface FileOperationResult { import { createLogger, type Logger } from '@repo/shared'; import type { Sandbox } from '../sandbox'; -const logger: Logger = createLogger({ - component: 'sandbox-do', - operation: 'openai-agent' -}); - // Helper functions for error handling function isErrorWithProperties(error: unknown): error is { message?: string; @@ -74,18 +69,24 @@ function toError(error: unknown): Error | undefined { export class Shell implements OpenAIShell { private cwd: string = '/workspace'; public results: CommandResult[] = []; + private readonly logger: Logger; - constructor(private readonly sandbox: Sandbox) {} + constructor(private readonly sandbox: Sandbox) { + this.logger = createLogger({ + component: 'sandbox-do', + operation: 'openai-shell' + }); + } async run(action: ShellAction): Promise { - logger.debug('SandboxShell.run called', { + this.logger.debug('SandboxShell.run called', { commands: action.commands, timeout: action.timeoutMs }); const output: ShellResult['output'] = []; for (const command of action.commands) { - logger.debug('Executing command', { command, cwd: this.cwd }); + this.logger.debug('Executing command', { command, cwd: this.cwd }); let stdout = ''; let stderr = ''; let exitCode: number | null = 0; @@ -105,7 +106,7 @@ export class Shell implements OpenAIShell { // Timeout would be indicated by a specific error or exit code outcome = { type: 'exit', exitCode }; - logger.debug('Command executed successfully', { + this.logger.debug('Command executed successfully', { command, exitCode, stdoutLength: stdout.length, @@ -114,14 +115,17 @@ export class Shell implements OpenAIShell { // Log warnings for non-zero exit codes or stderr output if (exitCode !== 0) { - logger.warn(`Command failed with exit code ${exitCode}`, { + this.logger.warn(`Command failed with exit code ${exitCode}`, { command, stderr }); } else if (stderr) { - logger.warn(`Command produced stderr output`, { command, stderr }); + this.logger.warn(`Command produced stderr output`, { + command, + stderr + }); } else { - logger.info(`Command completed successfully`, { command }); + this.logger.info(`Command completed successfully`, { command }); } } catch (error: unknown) { // Handle network/HTTP errors or timeout errors @@ -138,13 +142,13 @@ export class Shell implements OpenAIShell { errorMessage.includes('Timeout') || errorMessage.includes('timed out') ) { - logger.error(`Command timed out`, undefined, { + this.logger.error(`Command timed out`, undefined, { command, timeout: action.timeoutMs }); outcome = { type: 'timeout' }; } else { - logger.error(`Error executing command`, toError(error), { + this.logger.error(`Error executing command`, toError(error), { command, error: errorMessage || error, exitCode @@ -170,19 +174,19 @@ export class Shell implements OpenAIShell { exitCode: collectedExitCode, timestamp }); - logger.debug('Result collected', { + this.logger.debug('Result collected', { command, exitCode: collectedExitCode, timestamp }); if (outcome.type === 'timeout') { - logger.warn('Breaking command loop due to timeout'); + this.logger.warn('Breaking command loop due to timeout'); break; } } - logger.debug('SandboxShell.run completed', { + this.logger.debug('SandboxShell.run completed', { totalCommands: action.commands.length, resultsCount: this.results.length }); @@ -201,11 +205,17 @@ export class Shell implements OpenAIShell { */ export class Editor implements OpenAIEeditor { public results: FileOperationResult[] = []; + private readonly logger: Logger; constructor( private readonly sandbox: Sandbox, private readonly root: string = '/workspace' - ) {} + ) { + this.logger = createLogger({ + component: 'sandbox-do', + operation: 'openai-editor' + }); + } /** * Create a new file inside the sandbox by applying the provided diff. @@ -214,7 +224,7 @@ export class Editor implements OpenAIEeditor { operation: Extract ): Promise { const targetPath = this.resolve(operation.path); - logger.debug('WorkspaceEditor.createFile called', { + this.logger.debug('WorkspaceEditor.createFile called', { path: operation.path, targetPath }); @@ -223,12 +233,12 @@ export class Editor implements OpenAIEeditor { // Create parent directory if needed const dirPath = this.getDirname(targetPath); if (dirPath !== this.root && dirPath !== '/') { - logger.debug('Creating parent directory', { dirPath }); + this.logger.debug('Creating parent directory', { dirPath }); await this.sandbox.mkdir(dirPath, { recursive: true }); } const content = applyDiff('', operation.diff, 'create'); - logger.debug('Writing file content', { + this.logger.debug('Writing file content', { path: targetPath, contentLength: content.length }); @@ -242,7 +252,7 @@ export class Editor implements OpenAIEeditor { timestamp }; this.results.push(result); - logger.info('File created successfully', { + this.logger.info('File created successfully', { path: operation.path, timestamp }); @@ -259,7 +269,7 @@ export class Editor implements OpenAIEeditor { timestamp }; this.results.push(result); - logger.error('Failed to create file', toError(error), { + this.logger.error('Failed to create file', toError(error), { path: operation.path, error: errorMessage }); @@ -275,7 +285,7 @@ export class Editor implements OpenAIEeditor { operation: Extract ): Promise { const targetPath = this.resolve(operation.path); - logger.debug('WorkspaceEditor.updateFile called', { + this.logger.debug('WorkspaceEditor.updateFile called', { path: operation.path, targetPath }); @@ -283,12 +293,12 @@ export class Editor implements OpenAIEeditor { try { let original: string; try { - logger.debug('Reading original file', { path: targetPath }); + this.logger.debug('Reading original file', { path: targetPath }); const fileInfo = await this.sandbox.readFile(targetPath, { encoding: 'utf-8' }); original = fileInfo.content; - logger.debug('Original file read', { + this.logger.debug('Original file read', { path: targetPath, originalLength: original.length }); @@ -301,12 +311,12 @@ export class Editor implements OpenAIEeditor { errorMessage.includes('ENOENT') || errorObj.status === 404 ) { - logger.error('Cannot update missing file', undefined, { + this.logger.error('Cannot update missing file', undefined, { path: operation.path }); throw new Error(`Cannot update missing file: ${operation.path}`); } - logger.error('Error reading file', toError(error), { + this.logger.error('Error reading file', toError(error), { path: operation.path, error: errorMessage }); @@ -314,7 +324,7 @@ export class Editor implements OpenAIEeditor { } const patched = applyDiff(original, operation.diff); - logger.debug('Applied diff', { + this.logger.debug('Applied diff', { path: targetPath, originalLength: original.length, patchedLength: patched.length @@ -329,7 +339,7 @@ export class Editor implements OpenAIEeditor { timestamp }; this.results.push(result); - logger.info('File updated successfully', { + this.logger.info('File updated successfully', { path: operation.path, timestamp }); @@ -346,7 +356,7 @@ export class Editor implements OpenAIEeditor { timestamp }; this.results.push(result); - logger.error('Failed to update file', toError(error), { + this.logger.error('Failed to update file', toError(error), { path: operation.path, error: errorMessage }); @@ -361,7 +371,7 @@ export class Editor implements OpenAIEeditor { operation: Extract ): Promise { const targetPath = this.resolve(operation.path); - logger.debug('WorkspaceEditor.deleteFile called', { + this.logger.debug('WorkspaceEditor.deleteFile called', { path: operation.path, targetPath }); @@ -377,7 +387,7 @@ export class Editor implements OpenAIEeditor { timestamp }; this.results.push(result); - logger.info('File deleted successfully', { + this.logger.info('File deleted successfully', { path: operation.path, timestamp }); @@ -394,7 +404,7 @@ export class Editor implements OpenAIEeditor { timestamp }; this.results.push(result); - logger.error('Failed to delete file', toError(error), { + this.logger.error('Failed to delete file', toError(error), { path: operation.path, error: errorMessage }); diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 3e6f778a..9b3fb45c 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -22,7 +22,6 @@ import type { import { createLogger, getEnvString, - runWithLogger, type SessionDeleteResult, shellEscape, TraceContext @@ -905,48 +904,46 @@ export class Sandbox extends Container implements ISandbox { // Create request-specific logger with trace ID const requestLogger = this.logger.child({ traceId, operation: 'fetch' }); - return await runWithLogger(requestLogger, async () => { - const url = new URL(request.url); + const url = new URL(request.url); - // Capture and store the sandbox name from the header if present - if (!this.sandboxName && request.headers.has('X-Sandbox-Name')) { - const name = request.headers.get('X-Sandbox-Name')!; - this.sandboxName = name; - await this.ctx.storage.put('sandboxName', name); - } + // Capture and store the sandbox name from the header if present + if (!this.sandboxName && request.headers.has('X-Sandbox-Name')) { + const name = request.headers.get('X-Sandbox-Name')!; + this.sandboxName = name; + await this.ctx.storage.put('sandboxName', name); + } - // Detect WebSocket upgrade request (RFC 6455 compliant) - const upgradeHeader = request.headers.get('Upgrade'); - const connectionHeader = request.headers.get('Connection'); - const isWebSocket = - upgradeHeader?.toLowerCase() === 'websocket' && - connectionHeader?.toLowerCase().includes('upgrade'); + // Detect WebSocket upgrade request (RFC 6455 compliant) + const upgradeHeader = request.headers.get('Upgrade'); + const connectionHeader = request.headers.get('Connection'); + const isWebSocket = + upgradeHeader?.toLowerCase() === 'websocket' && + connectionHeader?.toLowerCase().includes('upgrade'); - if (isWebSocket) { - // WebSocket path: Let parent Container class handle WebSocket proxying - // This bypasses containerFetch() which uses JSRPC and cannot handle WebSocket upgrades - try { - requestLogger.debug('WebSocket upgrade requested', { - path: url.pathname, - port: this.determinePort(url) - }); - return await super.fetch(request); - } catch (error) { - requestLogger.error( - 'WebSocket connection failed', - error instanceof Error ? error : new Error(String(error)), - { path: url.pathname } - ); - throw error; - } + if (isWebSocket) { + // WebSocket path: Let parent Container class handle WebSocket proxying + // This bypasses containerFetch() which uses JSRPC and cannot handle WebSocket upgrades + try { + requestLogger.debug('WebSocket upgrade requested', { + path: url.pathname, + port: this.determinePort(url) + }); + return await super.fetch(request); + } catch (error) { + requestLogger.error( + 'WebSocket connection failed', + error instanceof Error ? error : new Error(String(error)), + { path: url.pathname } + ); + throw error; } + } - // Non-WebSocket: Use existing port determination and HTTP routing logic - const port = this.determinePort(url); + // Non-WebSocket: Use existing port determination and HTTP routing logic + const port = this.determinePort(url); - // Route to the appropriate port - return await this.containerFetch(request, port); - }); + // Route to the appropriate port + return await this.containerFetch(request, port); } wsConnect(request: Request, port: number): Promise { diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 7ecd5e4d..d4367084 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -24,9 +24,7 @@ export type { LogContext, Logger, LogLevel } from './logger/index.js'; export { createLogger, createNoOpLogger, - getLogger, LogLevelEnum, - runWithLogger, TraceContext } from './logger/index.js'; // Export all request types (enforce contract between client and container) diff --git a/packages/shared/src/logger/index.ts b/packages/shared/src/logger/index.ts index 26dd0729..6a50b7ac 100644 --- a/packages/shared/src/logger/index.ts +++ b/packages/shared/src/logger/index.ts @@ -2,7 +2,7 @@ * Logger module * * Provides structured, trace-aware logging with: - * - AsyncLocalStorage for implicit context propagation + * - Explicit logger passing via constructor injection * - Pretty printing for local development * - JSON output for production * - Environment auto-detection @@ -14,25 +14,14 @@ * // Create a logger at entry point * const logger = createLogger({ component: 'sandbox-do', traceId: 'tr_abc123' }); * - * // Store in AsyncLocalStorage for entire request - * await runWithLogger(logger, async () => { - * await handleRequest(); - * }); - * - * // Retrieve logger anywhere in call stack - * const logger = getLogger(); - * logger.info('Operation started'); + * // Pass to classes via constructor + * const service = new MyService(logger); * - * // Add operation-specific context + * // Create child loggers for additional context * const execLogger = logger.child({ operation: 'exec', commandId: 'cmd-456' }); - * await runWithLogger(execLogger, async () => { - * // All nested calls automatically get execLogger - * await executeCommand(); - * }); + * execLogger.info('Operation started'); * ``` */ - -import { AsyncLocalStorage } from 'node:async_hooks'; import { CloudflareLogger } from './logger.js'; import { TraceContext } from './trace-context.js'; import type { LogComponent, LogContext, Logger, LogLevel } from './types.js'; @@ -71,78 +60,6 @@ export function createNoOpLogger(): Logger { }; } -/** - * AsyncLocalStorage for logger context - * - * Enables implicit logger propagation throughout the call stack without - * explicit parameter passing. The logger is stored per async context. - */ -const loggerStorage = new AsyncLocalStorage(); - -/** - * Get the current logger from AsyncLocalStorage - * - * @throws Error if no logger is initialized in the current async context - * @returns Current logger instance - * - * @example - * ```typescript - * function someHelperFunction() { - * const logger = getLogger(); // Automatically has all context! - * logger.info('Helper called'); - * } - * ``` - */ -export function getLogger(): Logger { - const logger = loggerStorage.getStore(); - if (!logger) { - throw new Error( - 'Logger not initialized in async context. ' + - 'Ensure runWithLogger() is called at the entry point (e.g., fetch handler).' - ); - } - return logger; -} - -/** - * Run a function with a logger stored in AsyncLocalStorage - * - * The logger is available to all code within the function via getLogger(). - * This is typically called at request entry points (fetch handler) and when - * creating child loggers with additional context. - * - * @param logger Logger instance to store in context - * @param fn Function to execute with logger context - * @returns Result of the function - * - * @example - * ```typescript - * // At request entry point - * async fetch(request: Request): Promise { - * const logger = createLogger({ component: 'sandbox-do', traceId: 'tr_abc' }); - * return runWithLogger(logger, async () => { - * return await this.handleRequest(request); - * }); - * } - * - * // When adding operation context - * async exec(command: string) { - * const logger = getLogger().child({ operation: 'exec', commandId: 'cmd-123' }); - * return runWithLogger(logger, async () => { - * logger.info('Command started'); - * await this.executeCommand(command); // Nested calls get the child logger - * logger.info('Command completed'); - * }); - * } - * ``` - */ -export function runWithLogger( - logger: Logger, - fn: () => T | Promise -): T | Promise { - return loggerStorage.run(logger, fn); -} - /** * Create a new logger instance * diff --git a/packages/shared/tests/logger.test.ts b/packages/shared/tests/logger.test.ts index 8e60adba..6a4b47f1 100644 --- a/packages/shared/tests/logger.test.ts +++ b/packages/shared/tests/logger.test.ts @@ -18,7 +18,7 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { createLogger, getLogger, runWithLogger } from '../src/logger/index'; +import { createLogger } from '../src/logger/index'; import { CloudflareLogger } from '../src/logger/logger'; import { TraceContext } from '../src/logger/trace-context'; import type { LogContext } from '../src/logger/types'; @@ -420,87 +420,6 @@ describe('Logger Module', () => { }); }); - describe('AsyncLocalStorage Context', () => { - beforeEach(() => { - // Set NODE_ENV to production to force JSON output - process.env.NODE_ENV = 'production'; - }); - - afterEach(() => { - // Clean up - delete process.env.NODE_ENV; - }); - - it('should store and retrieve logger from context', async () => { - const logger = createLogger({ - component: 'durable-object', - traceId: 'tr_async' - }); - - await runWithLogger(logger, () => { - const retrievedLogger = getLogger(); - retrievedLogger.info('From async context'); - }); - - expect(consoleLogSpy).toHaveBeenCalledOnce(); - const output = JSON.parse(consoleLogSpy.mock.calls[0][0]); - expect(output.traceId).toBe('tr_async'); - }); - - it('should throw error when accessing logger outside context', () => { - expect(() => getLogger()).toThrow( - 'Logger not initialized in async context' - ); - }); - - it('should support nested runWithLogger calls', async () => { - const parentLogger = createLogger({ - component: 'durable-object', - traceId: 'tr_parent' - }); - - await runWithLogger(parentLogger, async () => { - const childLogger = getLogger().child({ operation: 'exec' }); - - await runWithLogger(childLogger, () => { - const retrievedLogger = getLogger(); - retrievedLogger.info('Nested context'); - }); - }); - - const output = JSON.parse(consoleLogSpy.mock.calls[0][0]); - expect(output).toMatchObject({ - traceId: 'tr_parent', - operation: 'exec' - }); - }); - - it('should isolate logger between async operations', async () => { - const logger1 = createLogger({ component: 'worker', traceId: 'tr_1' }); - const logger2 = createLogger({ component: 'worker', traceId: 'tr_2' }); - - const promise1 = runWithLogger(logger1, async () => { - await new Promise((resolve) => setTimeout(resolve, 10)); - getLogger().info('Logger 1'); - }); - - const promise2 = runWithLogger(logger2, async () => { - getLogger().info('Logger 2'); - }); - - await Promise.all([promise1, promise2]); - - expect(consoleLogSpy).toHaveBeenCalledTimes(2); - const outputs = consoleLogSpy.mock.calls.map((call) => - JSON.parse(call[0]) - ); - const traceIds = outputs.map((o) => o.traceId); - - expect(traceIds).toContain('tr_1'); - expect(traceIds).toContain('tr_2'); - }); - }); - describe('Edge Cases', () => { it('should handle empty context', () => { const logger = new CloudflareLogger( From 791ecf08e8a76a35bf76b07ea39f231dbf871ef6 Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 19 Nov 2025 17:03:14 +0000 Subject: [PATCH 2/3] Add changeset for logging cleanup --- .changeset/cleanup-unused-logging.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cleanup-unused-logging.md diff --git a/.changeset/cleanup-unused-logging.md b/.changeset/cleanup-unused-logging.md new file mode 100644 index 00000000..ecf1d48f --- /dev/null +++ b/.changeset/cleanup-unused-logging.md @@ -0,0 +1,5 @@ +--- +'@cloudflare/sandbox': patch +--- + +Remove unused logging infrastructure (getLogger, runWithLogger) that was never called From a4fb1357d6049b171ce7850fd6c98d73dab234d7 Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 19 Nov 2025 17:48:37 +0000 Subject: [PATCH 3/3] Fix e2e test worker template configuration Use 'vars' instead of 'env' for environment variables in wrangler template. --- tests/e2e/test-worker/wrangler.template.jsonc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/test-worker/wrangler.template.jsonc b/tests/e2e/test-worker/wrangler.template.jsonc index ca6e92a5..614b254c 100644 --- a/tests/e2e/test-worker/wrangler.template.jsonc +++ b/tests/e2e/test-worker/wrangler.template.jsonc @@ -5,7 +5,7 @@ "compatibility_flags": ["nodejs_compat"], "account_id": "b8afc92c7a87f699592038b756153d22", - "env": { + "vars": { "SANDBOX_LOG_LEVEL": "debug", "SANDBOX_LOG_FORMAT": "pretty" },