Skip to content
This repository was archived by the owner on Mar 27, 2026. It is now read-only.

Fix SandboxGetLogs in Typescript client#254

Merged
saltzm merged 12 commits intomainfrom
cursor/typescript-client-sandbox-logs-00e5
Feb 9, 2026
Merged

Fix SandboxGetLogs in Typescript client#254
saltzm merged 12 commits intomainfrom
cursor/typescript-client-sandbox-logs-00e5

Conversation

@saltzm
Copy link
Copy Markdown
Contributor

@saltzm saltzm commented Feb 6, 2026

Implement retry backoff for sandbox log fetches, make stdout/stderr lazy, and ensure cancelable iterators stop consumption to improve resource management and responsiveness.

The previous implementation eagerly started log streams (SandboxGetLogs) upon Sandbox object creation, even if stdout or stderr were never accessed. This PR changes stdout and stderr to be lazy-initialized, only starting the underlying gRPC stream when first accessed. Additionally, it adds exponential backoff to SandboxGetLogs retries to prevent hammering the server on transient errors and modifies stream cancellation to immediately stop consumption, preventing unnecessary resource usage. This also enables a previously skipped test that relied on lazy log streaming behavior.


Slack Thread

Open in Cursor Open in Web

…tor on cancel; lazy-init stdout/stderr to avoid eager SandboxGetLogs; enable docker mock test

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
@cursor
Copy link
Copy Markdown

cursor Bot commented Feb 6, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@cursor
Copy link
Copy Markdown

cursor Bot commented Feb 6, 2026

PR Summary

Medium Risk
Changes sandbox log streaming lifecycle (lazy init, cancellation, retry backoff), which can subtly affect stdout/stderr consumption timing and error behavior; new tests reduce but don’t eliminate behavioral risk across runtimes.

Overview
Improves SandboxGetLogs handling in the TS client by making Sandbox.stdout/Sandbox.stderr lazily initialized (log streams are no longer started during Sandbox construction) and wiring cancellation via AbortController/AbortSignal so reader cancellation stops the underlying gRPC stream.

Adds short exponential backoff and retry counter reset behavior to the sandboxGetLogs loop to avoid tight retry hammering on transient errors, and updates streamConsumingIter to propagate cancellation upstream via AsyncIterator.return() instead of draining the iterator. Re-enables the previously skipped experimental docker mock test and adds a focused sandbox_logs.test.ts suite covering laziness, retry/backoff, resume via lastEntryId, error propagation, and cancellation cleanup.

Written by Cursor Bugbot for commit bd129b4. This will update automatically on new commits. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Comment thread modal-js/src/sandbox.ts Outdated
Comment thread modal-js/src/streams.ts Outdated
Comment thread modal-js/src/streams.ts Outdated
Comment thread modal-js/src/sandbox.ts Outdated
Comment thread modal-js/src/sandbox.ts Outdated
…ts and honor AbortSignal during backoff

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Comment thread modal-js/src/streams.ts
Comment thread modal-js/src/streams.ts Outdated
…ents in streams.ts; keep onCancel, cancel semantics unchanged

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Comment thread modal-js/src/streams.ts Outdated
cursoragent and others added 3 commits February 6, 2026 21:43
…ram; update call sites

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
…guard and cleanup

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Comment thread modal-js/src/sandbox.ts Outdated
Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Comment thread modal-js/src/streams.ts
… reset, cancel)

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Comment thread modal-js/test/sandbox_logs.test.ts
cursoragent and others added 2 commits February 6, 2026 22:06
…ses setTimeout and format

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Comment thread modal-js/src/sandbox.ts Outdated
@saltzm saltzm requested a review from mwaskom February 6, 2026 22:18
@saltzm saltzm self-assigned this Feb 6, 2026
@saltzm saltzm marked this pull request as ready for review February 6, 2026 22:18
@saltzm saltzm changed the title Typescript client sandbox logs Fix SandboxGetLogs in Typescript client Feb 9, 2026
Comment thread modal-js/test/sandbox.test.ts
Copy link
Copy Markdown
Contributor

@ehdr ehdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overall looks good to me! Sent some comments over Slack too.

I believe I made the Go implementation lazy as well, but we don't have the backoff for retries, so I'd guess Go could also end up in a tight retry loop? So would be good to look into that as well!

…Iter JSDoc; switch tests to import from "modal"; add tests for non-retryable, retry exhaustion, and lastEntryId resumption

Co-authored-by: Matthew Saltz <saltzm@users.noreply.github.com>
Comment thread modal-js/test/sandbox_logs.test.ts
@saltzm saltzm requested a review from ehdr February 9, 2026 14:27
@saltzm saltzm enabled auto-merge (squash) February 9, 2026 14:45
@saltzm saltzm merged commit 239d52b into main Feb 9, 2026
6 checks passed
@saltzm saltzm deleted the cursor/typescript-client-sandbox-logs-00e5 branch February 9, 2026 14:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants