Skip to content

feat(sandbox): add idempotent retry for sandbox creation and connection - #457

Merged
miclle merged 6 commits into
qiniu:masterfrom
DROWNING2003:master
Jul 29, 2026
Merged

feat(sandbox): add idempotent retry for sandbox creation and connection#457
miclle merged 6 commits into
qiniu:masterfrom
DROWNING2003:master

Conversation

@DROWNING2003

Copy link
Copy Markdown
Contributor
  • Auto-generate UUID idempotency key + Idempotency-Key header for Create
  • Add _retryCall method wrapping Create and Connect, retries on 408/5xx server errors and network errors
  • Configurable maxRetries via maxRetries option or SANDBOX_RETRY_MAX env var (default 5)
  • Sandbox.create({ idempotencyKey }) optional parameter
  • Integration test for Git clone timeout retry
  • Example: examples/sandbox_idempotency_retry.js

- Add idempotency key auto-generation (crypto.randomUUID) with Idempotency-Key header
- Add _retryCall method wrapping create/connect with configurable maxRetries
- Retry on 408/5xx server errors and network errors (timeout, connection refused, etc.)
- Configurable via maxRetries option or SANDBOX_RETRY_MAX env var
- Add Sandbox.create({ idempotencyKey }) optional parameter
- Add integration test for Git clone timeout retry scenario
- Add sandbox_idempotency_retry example
- Add SANDBOX_RETRY_MAX to .env.example
@qiniu-prow qiniu-prow Bot added the size/L label Jul 29, 2026

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

Review Summary

Found five concrete issues affecting supported Node.js versions, public retry configuration and typings, test reliability, and example accuracy.

Validation note: reviewed and rechecked head 50eaa85c2af3847361d759f94a0e20e9fc4c6fb0. The focused suite could not run because the mounted workspace has no usable local test dependencies (mocha: Permission denied, then no local module).

View job run

Comment thread qiniu/sandbox/client.js
Comment thread qiniu/sandbox/client.js
Comment thread qiniu/sandbox/client.js
Comment thread test/sandbox_integration.test.js Outdated
Comment thread examples/sandbox_idempotency_retry.js Outdated
…rrors

- Add exponential backoff with jitter (500ms → 10s) between retries
- Integration test: re-throw non-retryable 4xx errors instead of logging
- Replace crypto.randomUUID() with crypto.randomBytes(16) for Node 6+ compat
- Add maxRetries to sandboxClientOptions() so Sandbox.create() respects it
- Add idempotencyKey to TypeScript SandboxCreateOptions
- Add maxRetries to TypeScript SandboxClientOptions
- Fix example to call Create twice with same idempotency key
…sandboxes

Previously sb1 was killed but sb2 leaked if the two Create calls returned
different sandbox IDs.
Comment thread qiniu/sandbox/client.js
…idempotency key stable

Check err.code for standard Node.js network error codes (ECONNREFUSED,
ECONNRESET, etc.) in addition to message-based detection. Add unit tests
for retry behavior and idempotency key stability across retries. Use
GIT_REPO_URL in integration test.

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

发现一处仍未覆盖的临时 DNS 错误。

Comment thread qiniu/sandbox/client.js
…AGAIN

generateIdempotencyKey now outputs RFC 9562 UUID v4 format.
normalizeMaxRetries validates non-integer/negative values from both
options and SANDBOX_RETRY_MAX environment variable. EAI_AGAIN added
to retryable network error codes. Remove git clone integration test.
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.79592% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.32%. Comparing base (32685e9) to head (7cd2a54).
⚠️ Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
qiniu/sandbox/client.js 91.48% 4 Missing ⚠️
qiniu/sandbox/sandbox.js 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #457      +/-   ##
==========================================
- Coverage   93.41%   93.32%   -0.10%     
==========================================
  Files          14       14              
  Lines        2006     2097      +91     
==========================================
+ Hits         1874     1957      +83     
- Misses        132      140       +8     
Files with missing lines Coverage Δ
qiniu/sandbox/sandbox.js 92.45% <50.00%> (-0.20%) ⬇️
qiniu/sandbox/client.js 95.56% <91.48%> (-1.43%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@miclle
miclle merged commit e80e588 into qiniu:master Jul 29, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants