fix(javascript): bound readiness polling sleeps by 7487 · Pull Request #1745 · opensandbox-group/OpenSandbox · GitHub
Skip to content

fix(javascript): bound readiness polling sleeps - #1745

Merged
Pangjiping merged 3 commits into
opensandbox-group:mainfrom
7487:fix/js-sdk-readiness-timeout
Sep 7, 2026
Merged

Pangjiping merged 3 commits into
opensandbox-group:mainfrom
7487:fix/js-sdk-readiness-timeout

Conversation

@7487

@7487 7487 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Go SDK: #1744. Kotlin / C# are left for separate PRs.

Testing

  • Not run (explain why)
  • Unit tests
  • Integration tests
  • e2e / manual verification

Added Sandbox.create readiness timeout does not overshoot by a polling interval in tests/sandbox.create.test.mjs (ping always returns false, timeout 20ms, interval 2s). Before the fix: expected timeout in ~20ms, took 2003ms; after: passes in ~22ms. pnpm run lint, pnpm run typecheck, and pnpm run test (129/129) pass in sdks/sandbox/javascript.

Breaking Changes

  • None
  • Yes (describe impact and migration path)

Checklist

  • Linked Issue or clearly described motivation
  • Added/updated docs (if needed)
  • Added/updated tests (if needed)
  • Security impact considered
  • Backward compatibility considered

🤖 Generated with Claude Code

waitUntilReady slept a full pollingIntervalMillis after every failed
health check, even when the remaining timeout budget was smaller. With
readyTimeoutSeconds=0.02 and healthCheckPollingInterval=2000 a
never-ready sandbox took ~2s to reject with SandboxReadyTimeoutException
instead of ~20ms.

Clamp the sleep to the remaining budget and exit once it is exhausted,
mirroring the Python fix in opensandbox-group#1734. The timeout message and attempt
count are unchanged.

Part of opensandbox-group#1739 (JavaScript SDK)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added sdk/js size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 7, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

@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: 43453103d9

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sdks/sandbox/javascript/src/sandbox.ts Outdated
After the clamped sleep wakes at exactly the deadline, a strict `>`
comparison still allowed one more health check past the budget. Use
`>=` so the loop exits at the deadline, matching the strict
before-deadline loops in the Python, Go, and Kotlin SDKs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@7487 7487 mentioned this pull request Sep 7, 2026
6 of 11 tasks

@Pangjiping Pangjiping left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@Pangjiping
Pangjiping merged commit 0af3a46 into opensandbox-group:main Sep 7, 2026
54 checks passed
@yanpgwang yanpgwang mentioned this pull request Sep 9, 2026
12 of 14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sdk/js size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants