Skip to content

fix(sdk-typescript): surface snapshot-from-sandbox capture failures#35

Open
mu-hashmi wants to merge 1 commit into
mainfrom
fix/sdk-snapshot-record-polling
Open

fix(sdk-typescript): surface snapshot-from-sandbox capture failures#35
mu-hashmi wants to merge 1 commit into
mainfrom
fix/sdk-snapshot-record-polling

Conversation

@mu-hashmi

@mu-hashmi mu-hashmi commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

sandbox._experimental_createSnapshot() used to watch only the sandbox
state. A snapshot-from-sandbox capture reverts the sandbox to its previous
state on both success and failure, so a failed capture was reported to
the caller as success while the snapshot never materialized (404 forever).

This PR makes the SDK wait on the snapshot record instead:

  • Resolve when the record reaches active.
  • Throw a DaytonaError carrying the record's errorReason when it reaches
    error / build_failed.
  • The capture route now documents a 409 when a snapshot with that name
    already exists for the organization; the response is propagated into the
    generated Go / Java / Python clients.

Sandbox takes an optional SnapshotsApi (defaulted from the API-targeted
client configuration, since the sandbox's own config is repurposed to point
at the toolbox proxy) so the poller queries the Daytona API directly.

Compatibility with servers that create the record only after success

The code refers to these as the "legacy API" path — a server that inserts
the snapshot record only after a successful capture, rather than up front
when the capture is accepted. Two guards keep the poller correct against
them:

  • Grace re-reads. The record insert and the sandbox-state revert happen
    in separate steps, so a poll can observe the reverted sandbox before the
    record lands. After seeing the sandbox leave snapshotting with no record
    yet, re-read the record up to 3 times, 500 ms apart, bounded by the caller
    timeout, before declaring the capture failed.
  • Stale-record rejection. These servers have no accept-time duplicate
    check, so polling by name could resolve against a pre-existing same-name
    snapshot while the real capture is still running. Reject any polled record
    whose createdAt predates the capture initiation beyond a 5-minute
    clock-skew allowance.

Behavior changes worth noting

  • A failed capture now surfaces as a thrown DaytonaError with the real
    error reason, instead of a silent success.
  • Attempting to capture into a name that already exists now returns 409
    from the capture route.

Test plan

  • nx test sdk-typescript — 16 suites / 215 tests green (adds coverage
    for success, capture failure, build failure, the legacy grace-read
    recovery and timeout paths, stale-record rejection, clock-skew tolerance,
    and the default SnapshotsApi wiring).
  • TypeScript type-only import check (tsc --verbatimModuleSyntax) green.
  • eslint sdk-typescript/src/**/*.ts — no new errors.
  • yarn generate:api-client — generated client drift limited to the
    additive 409 response (Go/Java/Python).
  • nx run sdk-typescript:docs — regenerated SDK reference reflects the
    new constructor argument and method contract.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Fixes silent successes when capturing a snapshot from a sandbox by polling the snapshot record until it’s active and surfacing real failures via DaytonaError. Also documents duplicate-name 409 and propagates it to generated clients.

  • Bug Fixes

    • _experimental_createSnapshot now waits on the snapshot record: resolves at active; throws DaytonaError with errorReason at error/build_failed.
    • Legacy server support: grace re-reads after sandbox leaves snapshotting; reject stale records created before the capture (5‑minute skew tolerance).
    • Sandbox constructor accepts optional SnapshotsApi (defaults to an API-targeted client) so polling hits the Daytona API, not the toolbox proxy.
    • After success, local sandbox data is refreshed best-effort.
  • Dependencies

    • OpenAPI: capture route documents 409 for duplicate snapshot names.
    • Regenerated clients (api-client-go, api-client-java, api-client-python, api-client-python-async) include the 409 response.
    • TypeScript SDK docs updated for the new SnapshotsApi arg and the snapshot method’s behavior.

Written for commit 67cbad8. Summary will update on new commits.

Review in cubic

…andbox

`_experimental_createSnapshot` previously watched only the sandbox state,
which reverts to its previous value on both success and failure of a
snapshot-from-sandbox capture. A failed capture was therefore reported as
success and the snapshot 404'd forever.

Wait on the snapshot record instead: resolve when it reaches `active`,
throw with the record's `errorReason` when it reaches `error`/`build_failed`.
The capture route now documents the 409 duplicate-name response, propagated
into the generated clients.

Compatibility with servers that only insert the record after a successful
capture ("legacy API" in the code) is handled by two guards:

- The record and the sandbox-state revert land in separate steps, so a poll
  can observe the reverted sandbox before the record exists. Re-read the
  record a few times, spaced out and bounded by the caller timeout, before
  declaring the capture failed.
- Such servers have no accept-time duplicate check, so polling by name could
  resolve against a pre-existing same-name snapshot. Reject records whose
  createdAt predates the capture initiation beyond a 5-minute skew allowance.

Sandbox gains an optional SnapshotsApi constructor argument (defaulted from
the API-targeted client config) so the poller hits the Daytona API rather
than the toolbox proxy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant