Skip to content

test(js-sdk): keep sending wake requests in the auto-resume test - #1865

Merged
mishushakov merged 3 commits into
mainfrom
devin/1789052072-flaky-auto-resume-test
Sep 10, 2026
Merged

test(js-sdk): keep sending wake requests in the auto-resume test#1865
mishushakov merged 3 commits into
mainfrom
devin/1789052072-flaky-auto-resume-test

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the flaky auto-resume wakes paused sandbox on http request test in packages/js-sdk/tests/sandbox/lifecyclePayload.test.ts (failure: https://github.com/e2b-dev/E2B/actions/runs/34489636300/job/102912738492expected 'paused' to equal 'running', sandbox still paused after 30s).

The JS test sent a single wake request (triggerAutoResume) and then waited for state === 'running'. If that one request raced the pause finalizing or was swallowed at the gateway (any non-timeout error response is a Response, not a throw, so a 5xx was silently accepted), nothing ever re-sent the wake signal and the test timed out.

-      await triggerAutoResume(url)          // one fetch, fire-and-forget
-      await waitForState(sandbox, 'running')
-      await waitForHttpStatus(url, 200)
+      await waitForHttpStatus(url, 200, { timeoutMs: 60_000 }) // re-sends the wake request until the guest answers
+      await waitForState(sandbox, 'running')

This mirrors what the Python sync/async test_auto_resume_wakes_on_http_request already do (wait_for_http_status(url, 200) before wait_for_state(RUNNING)). triggerAutoResume is removed as it is no longer used.

Per review, the waitForHttpStatus helper in tests/setup.ts now takes a trailing options object instead of a chain of optional positionals:

-waitForHttpStatus(url, expectedStatus, init?: RequestInit, timeoutMs = 30_000)
+waitForHttpStatus(url, expectedStatus, { init, timeoutMs = 30_000 }: { init?: RequestInit; timeoutMs?: number } = {})

Test-only change; no changeset needed. Verified locally against production (the test and a timed probe pass; wake → 200 → running happens within ~0.3s of the pause).

Link to Devin session: https://app.devin.ai/sessions/deb9199b540b49a889461e902787b9bd
Open in Devin Desktop: https://app.devin.ai/desktop/session/deb9199b540b49a889461e902787b9bd?variant=devin
Requested by: @mishushakov

Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@cla-bot cla-bot Bot added the cla-signed label Sep 10, 2026
@changeset-bot

changeset-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b25ab7c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TASTE.md review — checked the changed hunks in packages/js-sdk/tests/sandbox/lifecyclePayload.test.ts against the API-shape (T-3, T-9, T-11, T-12), timeout (T-44–T-47), and streaming/lifecycle (T-27, T-36) rules.

Violations on public SDK surface: 0. This PR is test-only and touches no exported API; it removes the one-shot triggerAutoResume helper and reuses the existing waitForHttpStatus poller, which also aligns the JS test with the Python sync/async test_auto_resume_wakes_on_http_request (T-1 parity in spirit).

Advisory (1, non-blocking): the new call site threads a positional undefined through the pre-existing waitForHttpStatus(url, status, init?, timeoutMs) chain — see inline. The helper's signature is not changed by this PR, so it is not counted as a violation.

Comment thread packages/js-sdk/tests/sandbox/lifecyclePayload.test.ts Outdated
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Package Artifacts

Built from 770bcf9. Download artifacts from this workflow run.

JS SDK (e2b@2.49.1-devin-1789052072-flaky-auto-resume-test.0):

npm install ./e2b-2.49.1-devin-1789052072-flaky-auto-resume-test.0.tgz

CLI (@e2b/cli@2.19.1-devin-1789052072-flaky-auto-resume-test.0):

npm install ./e2b-cli-2.19.1-devin-1789052072-flaky-auto-resume-test.0.tgz

Code Interpreter JS SDK (@e2b/code-interpreter@2.8.1-devin-1789052072-flaky-auto-resume-test.0):

npm install ./e2b-code-interpreter-2.8.1-devin-1789052072-flaky-auto-resume-test.0.tgz

Desktop JS SDK (@e2b/desktop@2.4.1-devin-1789052072-flaky-auto-resume-test.0):

npm install ./e2b-desktop-2.4.1-devin-1789052072-flaky-auto-resume-test.0.tgz

Python SDK (e2b==2.49.0+devin.1789052072.flaky.auto.resume.test):

pip install ./e2b-2.49.0+devin.1789052072.flaky.auto.resume.test-py3-none-any.whl

Code Interpreter Python SDK (e2b-code-interpreter==2.10.0+devin.1789052072.flaky.auto.resume.test):

pip install ./e2b_code_interpreter-2.10.0+devin.1789052072.flaky.auto.resume.test-py3-none-any.whl

Desktop Python SDK (e2b-desktop==2.5.0+devin.1789052072.flaky.auto.resume.test):

pip install ./e2b_desktop-2.5.0+devin.1789052072.flaky.auto.resume.test-py3-none-any.whl

Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
@mishushakov
mishushakov marked this pull request as ready for review September 10, 2026 14:59

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good — small, test-only change with sound logic.

What was reviewed: the waitForHttpStatus options-object signature change in tests/setup.ts and its single non-trivial call site update in network.test.ts; the lifecyclePayload.test.ts auto-resume flow rewrite (removed triggerAutoResume, now retries the wake request via waitForHttpStatus before checking state). Confirmed the other four call sites of waitForHttpStatus (network.test.ts, snapshot.test.ts, host.test.ts) only pass (url, status) and remain compatible with the new optional trailing options object. Checked the removed narrow catch in triggerAutoResume versus the helper's bare catch {} — not a fail-fast regression here since the helper's loop is bounded by timeoutMs and surfaces a descriptive timeout error either way.

Extended reasoning...

Overview

This PR touches only three test files in packages/js-sdk: tests/setup.ts (changes waitForHttpStatus's trailing parameters from positional (init?, timeoutMs) to an options object { init?, timeoutMs }), tests/sandbox/network.test.ts (updates its one call site to wrap headers inside the new init key), and tests/sandbox/lifecyclePayload.test.ts (removes the bespoke triggerAutoResume helper and replaces a fragile two-step wake-then-poll sequence with a single waitForHttpStatus(url, 200, { timeoutMs: 60_000 }) call that relies on the helper's existing retry loop to keep re-sending the wake request). No production/SDK-surface code is touched, so per CLAUDE.md no changeset is needed, and there is no Python-side mirroring obligation since no public SDK behavior changed.

Security risks

None. This is test infrastructure only — no auth, crypto, or permission-related code paths are affected, and no new external inputs or secrets are introduced.

Level of scrutiny

Low-to-moderate is appropriate given the small, mechanical, test-only diff, but I did verify call-site compatibility and the correctness of the auto-resume fix's premise (racing/lost wake requests get retried instead of relying on a single fire-and-forget request).

Other factors

The PR conversation shows two rounds of review feedback that were both addressed by follow-up commits from the author (mishushakov's request for an options-object signature was implemented in commit 19029fb, verifiable directly in the diff), and this run's own bug-hunting pass independently examined the concern about the removed fail-fast catch (bare catch {} in waitForHttpStatus) and ruled it out, since the retry loop is time-bounded and produces a clear timeout error regardless. With no outstanding findings and both raised concerns addressed either by a verifiable commit or by independent re-examination, and the change itself being small and low-risk, I'm confident this doesn't need further human review.

@mishushakov
mishushakov enabled auto-merge (squash) September 10, 2026 16:39
@mishushakov
mishushakov merged commit 1cd5cb8 into main Sep 10, 2026
42 checks passed
@mishushakov
mishushakov deleted the devin/1789052072-flaky-auto-resume-test branch September 10, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant