Merge 05b85e7 into 9710e56

Author: matthewlouisbrockman

packages/cli/src/commands/sandbox/exec.ts

@@ -197,22 +197,15 @@ async function sendStdin(sandbox: Sandbox, pid: number): Promise<void> {
       return
     }
 
-    // Fail fast instead of leaving a command blocked on stdin forever.
-    await killProcessBestEffort(sandbox, pid)
+    // Fail fast, and avoid leaking a process blocked on stdin.
+    try {
+      await sandbox.commands.kill(pid)
+    } catch (killErr) {
+      console.error(
+        'e2b: Failed to kill remote process after stdin EOF signaling failed.'
+      )
+      console.error(killErr)
+    }
     throw err
   }
 }
-
-async function killProcessBestEffort(
-  sandbox: Sandbox,
-  pid: number
-): Promise<void> {
-  try {
-    await sandbox.commands.kill(pid)
-  } catch (killErr) {
-    console.error(
-      'e2b: Failed to kill remote process after stdin EOF signaling failed.'
-    )
-    console.error(killErr)
-  }
-}

packages/cli/tests/commands/sandbox/backend_integration.test.ts

@@ -1,8 +1,13 @@
-import { spawn, spawnSync } from 'node:child_process'
-import path from 'node:path'
 import { afterAll, beforeAll, describe, expect, test } from 'vitest'
 import { Sandbox } from 'e2b'
 import { getUserConfig } from 'src/user'
+import {
+  bufferToText,
+  isDebug,
+  parseEnvInt,
+  runCli,
+  runCliWithPipedStdin,
+} from '../../setup'
 
 type UserConfigWithDomain = NonNullable<ReturnType<typeof getUserConfig>> & {
   domain?: string
@@ -16,21 +21,30 @@ const domain =
   userConfig?.domain ||
   'e2b.app'
 const apiKey = process.env.E2B_API_KEY || userConfig?.teamApiKey
+const shouldSkip = !apiKey || isDebug
+const integrationTest = test.skipIf(shouldSkip)
 const templateId =
   process.env.E2B_CLI_BACKEND_TEMPLATE_ID ||
   process.env.E2B_TEMPLATE_ID ||
   'base'
-const isDebug = process.env.E2B_DEBUG !== undefined
-const hasCreds = Boolean(apiKey)
-const shouldSkip = !hasCreds || isDebug
-const testIf = test.skipIf(shouldSkip)
-const cliPath = path.join(process.cwd(), 'dist', 'index.js')
 const sandboxTimeoutMs = parseEnvInt(
   'E2B_CLI_BACKEND_SANDBOX_TIMEOUT_MS',
   20_000
 )
 const perTestTimeoutMs = parseEnvInt('E2B_CLI_BACKEND_TEST_TIMEOUT_MS', 30_000)
 const spawnTimeoutMs = perTestTimeoutMs
+const cliEnv: NodeJS.ProcessEnv = {
+  ...process.env,
+  E2B_DOMAIN: domain,
+  E2B_API_KEY: apiKey,
+}
+
+delete cliEnv.E2B_DEBUG
+
+const runCliInSandbox = (args: string[]) =>
+  runCli(args, { timeoutMs: spawnTimeoutMs, env: cliEnv })
+const runCliWithPipeInSandbox = (args: string[], input: Buffer) =>
+  runCliWithPipedStdin(args, input, { timeoutMs: spawnTimeoutMs, env: cliEnv })
 
 describe('sandbox cli backend integration', () => {
   let sandbox: Sandbox
@@ -57,17 +71,21 @@ describe('sandbox cli backend integration', () => {
     }
   }, 15_000)
 
-  testIf('list shows the sandbox', { timeout: perTestTimeoutMs }, async () => {
-    const listResult = runCli(['sandbox', 'list', '--format', 'json'])
-    expect(listResult.status).toBe(0)
-    expect(sandboxExistsInList(listResult.stdout, sandbox.sandboxId)).toBe(true)
-  })
+  integrationTest(
+    'list shows the sandbox',
+    { timeout: perTestTimeoutMs },
+    async () => {
+      const listResult = runCliInSandbox(['sandbox', 'list', '--format', 'json'])
+      expect(listResult.status).toBe(0)
+      expect(sandboxExistsInList(listResult.stdout, sandbox.sandboxId)).toBe(true)
+    }
+  )
 
-  testIf(
+  integrationTest(
     'info shows the sandbox details',
     { timeout: perTestTimeoutMs },
     async () => {
-      const infoResult = runCli([
+      const infoResult = runCliInSandbox([
         'sandbox',
         'info',
         sandbox.sandboxId,
@@ -86,11 +104,11 @@ describe('sandbox cli backend integration', () => {
     }
   )
 
-  testIf(
+  integrationTest(
     'exec runs a command without piped stdin',
     { timeout: perTestTimeoutMs },
     async () => {
-      const execResult = runCli([
+      const execResult = runCliInSandbox([
         'sandbox',
         'exec',
         sandbox.sandboxId,
@@ -104,11 +122,11 @@ describe('sandbox cli backend integration', () => {
     }
   )
 
-  testIf(
+  integrationTest(
     'exec runs a command with piped stdin',
     { timeout: perTestTimeoutMs },
     async () => {
-      const pipedExecResult = await runCliWithPipedStdin(
+      const pipedExecResult = await runCliWithPipeInSandbox(
         ['sandbox', 'exec', sandbox.sandboxId, '--', 'sh', '-lc', 'wc -c'],
         Buffer.from('hello\n', 'utf8')
       )
@@ -120,27 +138,11 @@ describe('sandbox cli backend integration', () => {
     }
   )
 
-  /** Note: removing this test for now because it can be slow to get the logs causing tests to time out */
-  // testIf(
-  //   'logs returns successfully',
-  //   { timeout: perTestTimeoutMs },
-  //   async () => {
-  //     const logsResult = runCli([
-  //       'sandbox',
-  //       'logs',
-  //       sandbox.sandboxId,
-  //       '--format',
-  //       'json',
-  //     ])
-  //     expect(logsResult.status).toBe(0)
-  //   }
-  // )
-
-  testIf(
+  integrationTest(
     'metrics returns successfully',
     { timeout: perTestTimeoutMs },
     async () => {
-      const metricsResult = runCli([
+      const metricsResult = runCliInSandbox([
         'sandbox',
         'metrics',
         sandbox.sandboxId,
@@ -151,102 +153,24 @@ describe('sandbox cli backend integration', () => {
     }
   )
 
-  testIf(
+  integrationTest(
     'kill removes the sandbox',
     { timeout: perTestTimeoutMs },
     async () => {
-      const killResult = runCli(['sandbox', 'kill', sandbox.sandboxId])
+      const killResult = runCliInSandbox(['sandbox', 'kill', sandbox.sandboxId])
       expect(killResult.status).toBe(0)
 
       await assertSandboxNotListed(sandbox.sandboxId)
     }
   )
 })
 
-function runCli(
-  args: string[],
-  opts?: { input?: string | Buffer }
-): ReturnType<typeof spawnSync> {
-  const env: NodeJS.ProcessEnv = {
-    ...process.env,
-    E2B_DOMAIN: domain,
-    E2B_API_KEY: apiKey,
-  }
-  delete env.E2B_DEBUG
-
-  return spawnSync('node', [cliPath, ...args], {
-    env,
-    input: opts?.input,
-    encoding: 'utf8',
-    timeout: spawnTimeoutMs,
-  })
-}
-
-type PipeRunResult = {
-  status: number | null
-  stdout: Buffer
-  stderr: Buffer
-  error?: Error
-}
-
-function runCliWithPipedStdin(
-  args: string[],
-  input: Buffer
-): Promise<PipeRunResult> {
-  const env: NodeJS.ProcessEnv = {
-    ...process.env,
-    E2B_DOMAIN: domain,
-    E2B_API_KEY: apiKey,
-  }
-  delete env.E2B_DEBUG
-
-  return new Promise((resolve) => {
-    const child = spawn('node', [cliPath, ...args], {
-      env,
-      stdio: ['pipe', 'pipe', 'pipe'],
-    })
-
-    const stdoutChunks: Buffer[] = []
-    const stderrChunks: Buffer[] = []
-    let childError: Error | undefined
-    let timedOut = false
-
-    const timer = setTimeout(() => {
-      timedOut = true
-      child.kill()
-    }, spawnTimeoutMs)
-
-    child.stdout.on('data', (chunk) => stdoutChunks.push(Buffer.from(chunk)))
-    child.stderr.on('data', (chunk) => stderrChunks.push(Buffer.from(chunk)))
-    child.on('error', (err) => {
-      childError = err
-    })
-    child.on('close', (code) => {
-      clearTimeout(timer)
-      const timeoutError = timedOut
-        ? Object.assign(new Error('CLI command timed out'), {
-          code: 'ETIMEDOUT',
-        } as NodeJS.ErrnoException)
-        : undefined
-      resolve({
-        status: code,
-        stdout: Buffer.concat(stdoutChunks),
-        stderr: Buffer.concat(stderrChunks),
-        error: childError ?? timeoutError,
-      })
-    })
-
-    child.stdin.write(input)
-    child.stdin.end()
-  })
-}
-
 async function assertSandboxNotListed(sandboxId: string): Promise<void> {
   const retries = 10
   const delayMs = 500
 
   for (let i = 0; i < retries; i++) {
-    const listResult = runCli(['sandbox', 'list', '--format', 'json'])
+    const listResult = runCliInSandbox(['sandbox', 'list', '--format', 'json'])
     if (listResult.status === 0) {
       const exists = sandboxExistsInList(listResult.stdout, sandboxId)
       if (!exists) {
@@ -273,22 +197,6 @@ function sandboxExistsInList(
   return parsed.some((item) => item.sandboxId === sandboxId)
 }
 
-function bufferToText(value: Buffer | string | null | undefined): string {
-  if (!value) {
-    return ''
-  }
-  return typeof value === 'string' ? value : value.toString('utf8')
-}
-
-function parseEnvInt(name: string, fallback: number): number {
-  const raw = process.env[name]
-  if (!raw) {
-    return fallback
-  }
-  const parsed = Number.parseInt(raw, 10)
-  return Number.isFinite(parsed) ? parsed : fallback
-}
-
 function safeGetUserConfig(): ReturnType<typeof getUserConfig> | null {
   try {
     return getUserConfig()