Skip to content

Commit d09765b

Browse files
fix(sandbox): handle orphaned stream promise + preserve error detail
Two regressions from the previous timeout fix: - When the timeout won the race, the abandoned getSessionCommandLogs promise would reject on deleteSession with no handler (unhandledRejection). Attach a .catch that records the error and yields an 'error' outcome, so a late rejection is always handled. - The streaming catch dropped the thrown error, so failures before any chunks (env write, executeSessionCommand, missing cmdId) surfaced as empty output. Fall back to getErrorMessage(error) when nothing streamed. Adds conformance tests for the stream-reject and start-throw paths.
1 parent 7b693c1 commit d09765b

2 files changed

Lines changed: 48 additions & 9 deletions

File tree

apps/sim/lib/execution/remote-sandbox/conformance.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,4 +426,30 @@ describe('provider selection', () => {
426426
// The session is deleted to terminate the still-running command.
427427
expect(mockDeleteSession).toHaveBeenCalled()
428428
})
429+
430+
it('surfaces the thrown error when the stream rejects', async () => {
431+
useProvider('daytona')
432+
// A failure with no chunks streamed must still carry the error detail, not an
433+
// empty/opaque message.
434+
mockGetSessionCommandLogs.mockRejectedValue(new Error('session died'))
435+
436+
const result = await withPiSandbox((runner) =>
437+
runner.run('git clone ...', { timeoutMs: 1000, onStdout: () => {} })
438+
)
439+
440+
expect(result.exitCode).toBe(1)
441+
expect(result.stderr).toContain('session died')
442+
})
443+
444+
it('surfaces the error when starting the streamed command throws', async () => {
445+
useProvider('daytona')
446+
mockExecuteSessionCommand.mockRejectedValue(new Error('start failed'))
447+
448+
const result = await withPiSandbox((runner) =>
449+
runner.run('git clone ...', { timeoutMs: 1000, onStdout: () => {} })
450+
)
451+
452+
expect(result.exitCode).toBe(1)
453+
expect(result.stderr).toContain('start failed')
454+
})
429455
})

apps/sim/lib/execution/remote-sandbox/daytona.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,11 @@ class DaytonaSandboxHandle implements SandboxHandle {
156156
// and format failures from stderr, so returning empty strings here would
157157
// both break marker extraction and blank out error messages even though
158158
// the callbacks fired correctly.
159+
// The `.catch` keeps its own reference to the stream's outcome AND ensures a
160+
// late rejection is always handled — when the timeout wins the race, this
161+
// promise is abandoned and deleteSession (finally) tears the session down,
162+
// which rejects it; without the handler that would be an unhandledRejection.
163+
let streamError: unknown
159164
const streamed = this.sandbox.process
160165
.getSessionCommandLogs(
161166
sessionId,
@@ -170,6 +175,10 @@ class DaytonaSandboxHandle implements SandboxHandle {
170175
}
171176
)
172177
.then(() => 'done' as const)
178+
.catch((error: unknown) => {
179+
streamError = error
180+
return 'error' as const
181+
})
173182

174183
// `runAsync: true` returns immediately, so the timeout must be enforced here
175184
// — otherwise a hung command streams forever, unlike E2B's commands.run
@@ -179,23 +188,27 @@ class DaytonaSandboxHandle implements SandboxHandle {
179188
const timedOut = new Promise<'timeout'>((resolve) => {
180189
timer = setTimeout(() => resolve('timeout'), options.timeoutMs)
181190
})
191+
let outcome: 'done' | 'error' | 'timeout'
182192
try {
183-
const outcome = await Promise.race([streamed, timedOut])
184-
if (outcome === 'timeout') {
185-
return {
186-
stdout,
187-
stderr: stderr || `Command timed out after ${options.timeoutMs}ms`,
188-
exitCode: 124,
189-
}
190-
}
193+
outcome = await Promise.race([streamed, timedOut])
191194
} finally {
192195
if (timer) clearTimeout(timer)
193196
}
197+
if (outcome === 'timeout') {
198+
return {
199+
stdout,
200+
stderr: stderr || `Command timed out after ${options.timeoutMs}ms`,
201+
exitCode: 124,
202+
}
203+
}
204+
if (outcome === 'error') {
205+
return { stdout, stderr: stderr || getErrorMessage(streamError), exitCode: 1 }
206+
}
194207

195208
const finished = await this.sandbox.process.getSessionCommand(sessionId, commandId)
196209
return { stdout, stderr, exitCode: finished.exitCode ?? 0 }
197210
} catch (error) {
198-
return { stdout, stderr, exitCode: 1 }
211+
return { stdout, stderr: stderr || getErrorMessage(error), exitCode: 1 }
199212
} finally {
200213
try {
201214
await this.sandbox.process.deleteSession(sessionId)

0 commit comments

Comments
 (0)