Skip to content

test(js-sdk): run the full unit test suite in a browser - #1609

Open
mishushakov wants to merge 9 commits into
mainfrom
browser-test-suite
Open

test(js-sdk): run the full unit test suite in a browser#1609
mishushakov wants to merge 9 commits into
mainfrom
browser-test-suite

Conversation

@mishushakov

@mishushakov mishushakov commented Jul 24, 2026

Copy link
Copy Markdown
Member

Warning

The upstream CORS fix landed and rolled out — 13 of the 15 failures are gone. One infra gap remains.

The fix was revived in belt, not infra: belt#2068 "add CORS to synthesized responses and answer proxy-only preflights", merged 2026-08-24, live on production 2026-08-26. Verified directly — a nonexistent sandbox's synthesized 502 now carries access-control-allow-origin: *, and its preflight answers 204 with echoed headers.

needs blocks symptom
infra#3388 6 Sandbox.list({ limit }) pagination tests rolled out — these pass
belt#2068 13 stopped-sandbox tests rolled out — these pass
belt#2068 on the traffic-token path sandbox requires traffic access token TypeError: Failed to fetch on the plain GET at network.test.ts:152

The 15th failure was not an infra problem and is fixed in this PR (1964dde) — see the browser's connection-drop wording below. Confirmed on CI: the browser leg went 15 failures → 1.

Remaining gap, reproduced on production against a live token-gated sandbox:

$ curl -D - -H 'Origin: https://example.com' https://8080-<token-gated-sandbox>.e2b.app/
HTTP/2 403          # no access-control-allow-origin

$ curl -X OPTIONS -H 'Origin: ...' -H 'Access-Control-Request-Method: GET' ...
HTTP/2 403          # preflight refused rather than answered 204

belt#2068 lists traffic-token errors as covered by template.HandleError, and the destination-resolution path demonstrably has the fix, so this looks like an incomplete rollout rather than a missing change — the paths that resolve to a live sandbox (traffic-token 403, port-closed 502) still answer without CORS while the edge's destination-resolution 502 does. Worth confirming whether the node/orchestrator side of the rollout has completed.

Since the browser leg is not continue-on-error, it feeds the required SDK Tests Status check, so this stays red until the orchestrator rollout completes — deliberately un-gated, so the leg goes green on its own the moment it does. (A gate was tried and reverted: it worked, browser leg green, but it hides a real gap behind a skip.) Re-check with the curl above and re-run the leg.

Current browser leg: 1 failed file, 74 passed, 1 skipped (was 8 failed / 67 passed). Every other runtime leg is green on production; the two staging Node legs fail repo-wide right now (staging template builder returns template builder not found, and staging auto-pause loses sandboxes) — unrelated to this branch, reproducible on every recent run of any branch.

What

Promotes the browser leg from a single React smoke test to the full unit + connectionConfig suite running inside a real headless Chromium (@vitest/browser + Playwright) — the same coverage test:bun / test:deno / test:cf get. 76 files, 74 passing against production — one file still red on an upstream rollout (callout above), one skipped as debug-gated.

pnpm test:browser   # installs Chromium via pretest:browser, then runs the suite

React is gone as requested — the smoke test (tests/runtimes/browser/run.test.tsx) and its seven devDependencies (react, react-dom, @types/react, @types/react-dom, @testing-library/react, @vitejs/plugin-react, vitest-browser-react). Chromium installation moves off the default pretest hook onto pretest:browser, so plain pnpm test no longer downloads a browser.

The one thing the smoke test uniquely covered — the SDK driven with no process global at all, the way a real browser bundle runs — is kept as a dedicated test, since the rest of the suite runs against a process.env shim that would mask a regression there:

// tests/runtimes/browser/noProcessGlobal.test.ts
Reflect.deleteProperty(globalThis, 'process')
const sandbox = await Sandbox.create(template, { apiKey, domain })
await sandbox.files.write('hello.txt', 'Hello World')

Failure screenshots are off (browser.screenshotFailures: false): these tests never render anything, so vitest's default would write a PNG of a blank page per failed test into .vitest-attachments/.

Test bugs the leg surfaced

  1. Two assertions were vacuous. commands/kill and pty/kill asserted rejects.toThrowError(ProcessExitError) — but ProcessExitError has never been exported from src. Vite's SSR transform resolves the missing named import to undefined, so toThrowError(undefined) asserted nothing on any runtime; real ESM in the browser turns it into a hard SyntaxError. Now CommandExitError.

  2. files/read.test.ts ran three tests twice — a byte-identical duplicated block, provisioning a second real sandbox for each on every runtime leg.

  3. A cold-cache-only flake, root-caused. api/http2 and envd/http2 carried dead vi.doUnmock('undici') / vi.doUnmock('../../src/utils') calls from an old refactor (neither file mocks anything any more). The bare 'undici' one made Vite discover and pre-bundle a Node HTTP client mid-run, reloading the browser page under the running suite and breaking whichever file was being collected. Verified: 1-in-4 failure before, three cold-cache runs green after.

Node-only test APIs are replaced with cross-runtime equivalents: node:crypto randomUUID/createHash → WebCrypto, BufferTextEncoder/TextDecoder, path.basename → inline.

Two real browser bugs found

Both verified over the wire; neither was fixable in the SDK alone.

Pagination silently truncated — the cursor lives in the x-next-token response header, which the API didn't name in Access-Control-Expose-Headers, so a browser withheld it and every page looked like the last:

const paginator = Sandbox.list({ limit: 10 })
const all = []
while (paginator.hasNext) all.push(...(await paginator.nextItems()))
// Node: all 200 sandboxes.  Browser (before the fix): 10, hasNext === false, no error.

Fixed upstream in infra#3388, rolled out 2026-07-24 — the six tests are un-gated here and pass. SDK-293.

SDK-294 — isRunning() threw instead of returning false. A stopped sandbox's 502 came from the edge without CORS headers, so the probe rejected opaquely (running: 204 + ACAO: * / paused: 502 + no ACAO). Because a browser always preflights the SDK's envd requests (they carry E2b-Sandbox-Id/E2b-Sandbox-Port) and a preflight needs an ok status, adding the header to the 502 alone wasn't enough — the OPTIONS path needed a 2xx too. Fixed upstream in belt#2068 and confirmed on production: isRunning() now returns false for a stopped sandbox in the browser, and the 13 tests that depended on it pass.


The kill-mid-request error was a separate, SDK-side bug — originally filed under SDK-294 as another symptom of the missing CORS headers. It isn't: belt#2068 rolled out, the other 13 went green, and killing the sandbox while a command is running throws an actionable error still failed with SandboxError: 2: [unknown] network error. When the connection drops mid-request the SDK probes the sandbox's health to tell a killed sandbox from a transient blip, but the probe is gated on the connection-dropped message, whose wording is runtime-specific — and the browser's was missing:

const CONNECTION_TERMINATED_MESSAGES = [
  'terminated',                                     // Node (undici)
  'The socket connection was closed unexpectedly',  // Bun
  'error reading a body from connection',           // Deno
  'Network connection lost',                        // Cloudflare Workers
  'network error',                                  // Browser  ← added
]

So the probe never ran and the error stayed generic. The fragment is broad, which is fine by construction: matching only earns a health probe, and the error stays generic unless the probe confirms the sandbox is gone. A browser reports every failed request identically on purpose, so this is the only wording available. Both entry points (handleRpcErrorWithHealthCheck for RPC, handleEnvdApiFetchError for the envd HTTP API) share the predicate, so one edit covers both; the per-runtime tables in handleRpcError.test.ts and handleEnvdApiError.test.ts gain a Browser row. No Python counterpart — it detects this by exception type, not message wording, and has no browser runtime.

belt#2068 also fixes the content negotiation the investigation turned up: the proxy chose HTML over JSON by User-Agent sniffing, and browser fetch can't override its UA, so an SDK call from a browser got the HTML error page where it expects JSON (Accept was ignored). That is un-gated here too.

How limitations are handled

No capability flags — nothing is skipped for being a browser. The one still-failing test is left red on purpose, waiting on the upstream rollout rather than gated around; see the callout at the top. The five tests that read a response from a server the test itself starts in the sandbox now start a CORS-enabled one, via corsHttpServerCmd in tests/setup.ts:

// was: 'python3 -m http.server 8000' — no CORS headers, so a browser hands the
// test an opaque `TypeError: Failed to fetch` instead of the response
await sandbox.commands.run(corsHttpServerCmd(8000), { background: true })

This was originally gated behind a canFetchSandboxServers flag as an inherent browser limitation. It isn't one: the server is the test's own, and a real browser app's server opts into CORS exactly the same way — so the gate was hiding coverage of getHost and the proxy path rather than documenting a wall. Access-Control-Allow-Headers plus do_OPTIONS also covers the traffic-access-token test, whose custom header the browser preflights.

Confirmed on the browser leg: of the five tests this un-gates, four now passping server in running sandbox, sandbox works without token, auto-resume wakes paused sandbox on http request, and pause and resume a sandbox with http server (that last one needed belt#2068, since it calls isRunning() on a paused sandbox). That's the CORS server working end-to-end through the proxy against production. The fifth, sandbox requires traffic access token, reads a proxy-synthesized 403 after a preflight that can't carry the token — both of which belt#2068 handles, on the orchestrator side that hasn't rolled out yet.

The exclude list is only for genuinely Node-only suites (tests/bundle/** reads dist via node:fs, tests/undici.test.ts resolves packages off process.versions.node) plus the two msw suites, whose msw/node entry pulls in node:http and can't be served to a browser. Porting those needs setupWorker plus a service worker from a public dir — tracked as a follow-up on SDK-292, and worth doing since cancellation is the most runtime-divergent part of this SDK.

A real bug the leg caught

main added toUploadBody leaves an async-iterable foreign stream alone while this branch was in review, and it failed in Chromium with expected '[object ReadableStream]' to be 'hello'. Not a test artifact — the mechanism, confirmed directly:

const asyncIterable = { async *[Symbol.asyncIterator]() { yield new TextEncoder().encode('hello') } }
await new Response(asyncIterable).text()
// Node:     'hello'                    (undici accepts any async iterable)
// Chromium: '[object ReadableStream]'   (the web platform does not)

toDispatchableStream treats async-iterability as "the platform will accept this", which is an undici extension rather than web behavior. So buffering a stream via new Response(stream) in a browser stringified it, and this silently uploaded 23 bytes of text instead of the file:

import { ReadableStream } from 'web-streams-polyfill' // or any replaced global / other realm

await sandbox.files.write('data.bin', new ReadableStream({ /* ... */ }))
// Before: the file contains the literal text "[object ReadableStream]"
// After:  the file contains the data

The fix drains the stream through its reader — the one part every implementation shares — instead of guessing what the platform accepts. Note the obvious fix is wrong: skipping the async-iterable branch in the browser re-wraps a native stream when globalThis.ReadableStream has been replaced by a polyfill, which is precisely what that branch exists to prevent (there's a test for it). Verified in Chromium and Node.

CI

New browser matrix leg on ubuntu-22.04, with the Playwright cache moved onto it (no other leg needs Chromium any more). The leg is dropped by node-only staging callers, and it's the only leg whose result depends on the API's CORS headers — the matrix comment now says so, rather than claiming the non-Node legs add no backend signal.

Notes

  • No longer test-only: carries two source fixes, each with a patch changeset — the toBlob stream drain (below) and the browser connection-drop wording (above). Everything else is tests and CI.
  • Rebased onto main twice. The first time, Playwright had been extracted into a playwright:install script scoped to the node leg — pretest:browser now delegates to it rather than duplicating the install — and six new msw/node suites were added, which can't be served to a browser and are excluded. The second time conflicted on packages/js-sdk/package.json and pnpm-lock.yaml against chore: remove SDK example files and their package.json scripts #1769 (which removed the example files): resolved to main's newer dev-dep versions minus the React deps this branch drops, with the lockfile reset to main's and regenerated rather than hand-merged, so its only delta from main is the React tree. Note the Playwright bump to ^1.62 invalidates a local Chromium cache — pnpm run playwright:install; CI is unaffected.
  • vi.stubEnv and delete process.env.X keep working in the browser because the shim aliases process.env to import.meta.env, which is where vitest puts config env and where stubEnv writes. Verified E2B_DEBUG=1 pnpm test:browser correctly skips the debug-gated tests.
  • Verification for the CORS server, since a browser-side change like this is easy to get subtly wrong: ran a local instance of the exact generated command and fetched it cross-origin (different port = different origin) from real Chromium — simple GET readable, and a custom-header request survives the preflight. Negative control against a plain python -m http.server reproduces the TypeError: Failed to fetch the browser leg was showing, so the probe is meaningful rather than vacuous. Also checked there's exactly one Access-Control-Allow-Origin header — a duplicate would make browsers reject the response.
  • Review feedback addressed: noProcessGlobal.test.ts imports the SDK dynamically inside the test body — a static import is evaluated at collection time, while the process shim is still installed, so it only covered the call path and not module evaluation. (An earlier revision also put the canFetchSandboxServers gate on pause and resume a sandbox with http server; that gate was deleted wholesale later in the branch when the tests moved to corsHttpServerCmd.)
  • No new type errors: 22 pre-existing test type errors before and after (the 2 that went away are the ProcessExitError ones this fixes).
  • Not addressed, pre-existing, flagged in case you want a follow-up: tests/api/list.test.ts duplicates five tests byte-for-byte (only the three betaPause ones intentionally differ, instance vs static), and files/write.test.ts:130 repeats write file as a strict subset of the test at :6 — together ~7 redundant sandboxes per run per leg. Left alone rather than deleting tests outside this change's scope.

Closes SDK-292

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Jul 24, 2026

Copy link
Copy Markdown

SDK-292

SDK-293

@cla-bot cla-bot Bot added the cla-signed label Jul 24, 2026
@cursor

cursor Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR Summary

Medium Risk
Medium risk from changes to upload blob conversion and connection-drop error handling in production browser paths, plus a new gating CI leg that depends on API CORS for one remaining upstream rollout.

Overview
Adds a dedicated browser CI leg and pnpm test:browser that runs the shared unit + connectionConfig suites in headless Chromium (replacing the old React smoke test and dropping React-related devDependencies). Playwright install moves to pretest:browser so default pnpm test no longer pulls Chromium.

Two SDK patches: toBlob now drains ReadableStream bodies through getReader() instead of new Response(stream), fixing browser uploads that silently sent [object ReadableStream]. isConnectionTerminatedMessage includes the browser’s network error fragment so a sandbox killed mid-request surfaces TimeoutError instead of a generic SandboxError.

Test harness changes make shared suites portable: corsHttpServerCmd for cross-origin fetches from sandbox HTTP servers, TextEncoder/TextDecoder and WebCrypto instead of Buffer/node:crypto, CommandExitError in kill tests ( ProcessExitError was never exported), removal of dead vi.doUnmock calls that caused browser flakes, and a noProcessGlobal test that loads the SDK without process.

Reviewed by Cursor Bugbot for commit 3288bcd. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Package Artifacts

Built from 7a4755c. Download artifacts from this workflow run.

JS SDK (e2b@2.46.1-browser-test-suite.0):

npm install ./e2b-2.46.1-browser-test-suite.0.tgz

CLI (@e2b/cli@2.18.1-browser-test-suite.0):

npm install ./e2b-cli-2.18.1-browser-test-suite.0.tgz

Python SDK (e2b==2.46.0+browser.test.suite):

pip install ./e2b-2.46.0+browser.test.suite-py3-none-any.whl

Comment thread packages/js-sdk/tests/sandbox/snapshot.test.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5626cfff60

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/js-sdk/tests/runtimes/browser/noProcessGlobal.test.ts Outdated

@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.

LGTM — test-only PR that expands the browser CI leg to the full unit suite, with no production code changes.

Extended reasoning...

Overview

This PR touches only test infrastructure: the browser vitest config, test files (replacing Node-only APIs like Buffer/node:crypto with WebCrypto/TextEncoder), the CI workflow matrix, package.json devDependencies (dropping React test tooling), and pnpm-lock.yaml. No files under packages/js-sdk/src/ are modified.

Security risks

None. No auth, crypto business logic, or user-facing behavior changes — only test harness and CI configuration.

Level of scrutiny

Low-to-moderate scrutiny is appropriate: this is a test-only change with no production code path affected. I spot-checked the PR's own claims against src/: CommandExitError is indeed exported (ProcessExitError never existed, confirming the described vacuous-assertion bug fix), and the browser-specific skip flags (canFetchSandboxServers, canReadPaginationToken, canObserveStoppedSandbox) correctly mirror existing runtime !== 'browser' CORS-avoidance logic already present in src/connectionConfig.ts. The new capability-gated skips are a reasonable way to surface browser limitations without silently dropping coverage.

Other factors

The bug hunting system found no issues. The PR description is thorough and self-documents the rationale for each test change (duplicate test removal, flaky mock cleanup, Node-API replacements). No changeset is needed since this is test-only, consistent with prior similar PRs (#1600) per CLAUDE.md guidance.

@changeset-bot

changeset-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3288bcd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
e2b Patch

Not sure what this means? Click here to learn what changesets are.

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

Comment thread packages/js-sdk/tests/setup.ts Outdated
Comment thread packages/js-sdk/tests/setup.ts Outdated
mishushakov and others added 7 commits August 26, 2026 15:08
Promotes the browser leg from a single React smoke test to the full unit +
connectionConfig suite running inside a real headless Chromium via
@vitest/browser + Playwright (`pnpm test:browser`) — the same coverage
test:bun / test:deno / test:cf get. 69 files / 343 tests green.

The React smoke test and its devDependencies are gone (react, react-dom,
@types/react{,-dom}, @testing-library/react, @vitejs/plugin-react,
vitest-browser-react). The one thing it uniquely covered — the SDK driven with
no `process` global at all, as in a real browser bundle — is kept as
tests/runtimes/browser/noProcessGlobal.test.ts. Chromium installation moves
from the default `pretest` hook to `pretest:browser`, so plain `pnpm test` no
longer downloads a browser.

Test bugs the leg surfaced:

- commands/kill and pty/kill asserted `rejects.toThrowError(ProcessExitError)`,
  but `ProcessExitError` is not exported from src. Vite's SSR transform
  resolved the named import to `undefined`, so both assertions were vacuous;
  real ESM in the browser makes it a hard SyntaxError. Now CommandExitError.
- files/read.test.ts had three byte-identical duplicated tests, each
  provisioning a second sandbox on every runtime.
- api/http2 and envd/http2 carried dead `vi.doUnmock('undici')` /
  `vi.doUnmock('../../src/utils')` calls from an old refactor. The bare
  'undici' one made Vite pre-bundle a Node HTTP client mid-run and reload the
  page under the running suite — a cold-cache-only flake that broke whichever
  file was being collected at the time.

Node-only test APIs are replaced with cross-runtime equivalents: node:crypto
randomUUID/createHash to WebCrypto, Buffer to TextEncoder/TextDecoder,
path.basename inlined.

Limitations a browser physically can't work around are gated by documented
capability flags in tests/setup.ts, so they report as skipped instead of
disappearing into the config's exclude list. Two are real user-facing bugs,
filed as SDK-293 (x-next-token is not in Access-Control-Expose-Headers, so
Sandbox.list pagination silently truncates) and SDK-294 (the stopped-sandbox
502 has no CORS headers, so isRunning() throws instead of returning false).

Closes SDK-292

Co-Authored-By: Claude <noreply@anthropic.com>
These tests drive the SDK and never render anything, so vitest's
browser.screenshotFailures (on by default when headless) wrote a PNG of a blank
page per failed test into .vitest-attachments/. Turned off, which also lets the
.gitignore entry it needed go away.

Removes canReadPaginationToken: the API not exposing X-Next-Token via
Access-Control-Expose-Headers is fixed upstream in infra#3388, so the six
limit-based pagination tests are un-gated and run in the browser like
everywhere else.

Note infra#3388 is not on production yet, and the browser leg only ever runs
against production (staging callers pass node-only, which drops it), so those
six tests fail on `paginator.hasNext` until the deploy lands.

Co-Authored-By: Claude <noreply@anthropic.com>
Removes canObserveStoppedSandbox. The stopped-sandbox 502 arriving from the
edge without CORS headers is fixed in e2b-dev/infra#3389, so isRunning() can
report false in a browser again and checkSandboxHealth can return false, which
restores the actionable TimeoutError on the 24 call sites that use it.

Also un-gates host.test.ts 'ping server in non-running sandbox', which was
under the wrong flag: it asserts the edge's own 502 JSON envelope rather than a
response from a server inside the sandbox, so infra#3389 covers it too (the
same PR's content-negotiation fix is what keeps the body JSON rather than the
HTML error page). Its sibling 'ping server in running sandbox' does read the
user's python server, so that one keeps the gate.

canFetchSandboxServers stays for the four tests that read a response from a
server the test started in the sandbox — no CORS headers there, and one of them
sends a custom header that would preflight into python's 501. Its doc comment
no longer cites the proxy 502 as an example, since that now carries the header.

Until infra#3389 reaches production these 14 tests fail on the browser leg —
13 with 'TypeError: Failed to fetch' and one with the degraded SandboxError in
place of TimeoutError, matching the two documented consequences exactly. The
Node/Bun/Deno/workerd legs are unaffected (7 files / 26 tests green on Node).

Co-Authored-By: Claude <noreply@anthropic.com>
- gate the http-server pause/resume test on canFetchSandboxServers: it
  fetches a `python -m http.server` the test starts itself, which sends no
  CORS headers. Masked today by the isRunning failure, it would have started
  failing the moment infra#3389 rolls out.
- import the SDK dynamically in noProcessGlobal, after the process shim is
  deleted. A static import is evaluated at collection time while the shim is
  still installed, so a top-level `process` read anywhere in the module graph
  would have passed here and still crashed a browser app on import.

Co-Authored-By: Claude <noreply@anthropic.com>
…browser gate

`python -m http.server` sends no CORS headers, so a browser handed the test an
opaque `TypeError: Failed to fetch` instead of the response. That was gated
behind `canFetchSandboxServers` as inherent, but it isn't: the server is ours,
and a real browser app's own server opts in the same way. `corsHttpServerCmd`
starts one that does, so the five tests run in Chromium too and the last
capability flag is gone.

Verified in Chromium against a local instance of the exact generated command:
cross-origin GET readable, and a custom-header request survives the preflight
via do_OPTIONS. Negative control against a plain `python -m http.server`
reproduces the `Failed to fetch` the browser leg was showing.

Co-Authored-By: Claude <noreply@anthropic.com>
… was written

Six new suites import msw's `setupServer`, whose `msw/node` entry pulls in
node:http and can't be served to a browser. Also drops ENABLE_VOLUME_TESTS from
the forwarded env — main removed that flag when it mocked the volume tests.

Co-Authored-By: Claude <noreply@anthropic.com>
Buffering a ReadableStream went through `new Response(stream)`, whose accepted
body types differ per runtime: undici takes any async iterable, a browser takes
only its own stream class and stringifies the rest. So a browser uploading a
stream from a polyfill or another realm silently sent the 23-byte string
"[object ReadableStream]" instead of the file.

Draining through the reader — the one part every implementation shares — is
correct everywhere and drops the runtime-dependent guess. Note the obvious fix
(skip the async-iterable branch of `toDispatchableStream` in the browser) is
wrong: it re-wraps a native stream when `globalThis.ReadableStream` has been
replaced by a polyfill, which is what that branch exists to prevent.

Found by the browser test leg via `toUploadBody leaves an async-iterable
foreign stream alone`, which main added while this branch was in review.

Co-Authored-By: Claude <noreply@anthropic.com>
@mishushakov
mishushakov force-pushed the browser-test-suite branch 2 times, most recently from ec0622a to c8387f1 Compare August 26, 2026 13:36

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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 using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c8387f1. Configure here.

Comment thread packages/js-sdk/tests/sandbox/network.test.ts
Comment thread packages/js-sdk/src/envd/rpc.ts
mishushakov and others added 2 commits August 26, 2026 15:44
When a sandbox is killed while a request is in flight the SDK probes the
sandbox's health to tell that apart from a transient network failure, and
returns an actionable TimeoutError when the probe confirms the sandbox is gone.
The probe is gated on the connection-dropped message, whose wording is
runtime-specific — and the browser's, `network error`, was missing, so killing a
sandbox mid-command surfaced a generic `SandboxError: [unknown] network error`.

The fragment is broad, which is fine by construction: matching only earns a
health probe, and the error stays generic unless the probe confirms the sandbox
is gone. A browser reports every failed request identically on purpose, so this
is the only wording available.

Found by the browser test leg, which attributed this to the missing CORS headers
on proxy-synthesized responses. It wasn't that: belt#2068 rolled out to
production and the other 13 failures went green, this one didn't.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
infra#3389 was closed unmerged; the fix that landed is belt#2068. Note also
which paths are still awaiting the orchestrator rollout, since that is what
keeps `sandbox requires traffic access token` red on the browser leg.

Comment only — the test stays un-gated deliberately, so the leg goes green on
its own once the rollout completes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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