Repository navigation
[wrangler] Retry transient failures in workflows instances list and describe - #15871
Conversation
🦋 Changeset detectedLatest commit: 2cbe123 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
@cloudflare/containers-shared
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/runtime-types
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
The two failing CI jobs look unrelated to this change. This PR only touches
I don't have permission to rerun the jobs from a fork. Could a maintainer rerun the failed jobs when convenient? Thanks! |
dario-piotrowicz
left a comment
There was a problem hiding this comment.
Thanks @tw4 for this fix! 🙏
The changes here look good to me but as I suggested I would rename the new utility and add dedicated unit tests for it, could you have a go at it? 🙏
|
Codeowners approval required for this PR:
Show detailed file reviewers |
|
@tw4 the two red CI jobs keep failing, could you rebase your branch to make sure that this aren't due to your branch being out-of-sync with |
…escribe A single 5xx response or dropped connection made these read-only commands fail even when the next request would succeed. Route their reads, and the read that resolves `--id latest`, through the existing retryOnAPIFailure helper. Each attempt gets its own copy of the query params because fetchCursorPage deletes `page` from them, and under --json retry notices go to stderr so stdout stays valid JSON.
delete built its own args object for getInstanceIdFromArgs and dropped `json`, so a Retry-After notice could land on stdout ahead of the JSON result. Make `json` a required key so callers can't omit it silently.
Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com>
Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com>
48df2a2 to
2cbe123
Compare
|
@dario-piotrowicz thanks for the review! I've rebased the branch on the latest I also addressed your review:
|
Great! Thank you very much! 🙏 Let's ship this! 😄 |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
## pnpm-workspace.yaml (default) ## Dependency Updates | Package | From | To | Type | | --- | --- | --- | --- | | `wrangler` | 4.146.0 | 4.147.0 | minor | ## Release Notes <details> <summary><b>wrangler</b> (4.146.0 → 4.147.0)</summary> ### Minor Changes - [#15928](cloudflare/workers-sdk#15928) [`7f57b1c`](cloudflare/workers-sdk@7f57b1c) Thanks [@<!---->ichernetsky-cf](https://github.com/ichernetsky-cf)! - Allow `"us"` as a jurisdiction for Container applications Container placement constraints now accept `constraints.jurisdiction: "us"` in Wrangler and typed Cloudflare configuration. This makes the US jurisdiction available alongside `"eu"` and `"fedramp"`. ### Patch Changes - [#15974](cloudflare/workers-sdk#15974) [`7f700ef`](cloudflare/workers-sdk@7f700ef) Thanks [@<!---->martinezjandrew](https://github.com/martinezjandrew)! - Fix `wrangler containers list` to report live instances The `LIVE INSTANCES` column now reports each application's active runtime instances instead of its configured instance count, matching the Cloudflare dashboard. JSON output continues to expose the configured count through the existing `instances` field. - [#15980](cloudflare/workers-sdk#15980) [`90e6a1b`](cloudflare/workers-sdk@90e6a1b) Thanks [@<!---->martinezjandrew](https://github.com/martinezjandrew)! - Accept Durable Object application IDs in Containers commands `wrangler containers instances` and `wrangler containers delete` now accept the 32-character hexadecimal application IDs returned for Durable Object-backed applications, in addition to legacy dashed UUIDs. - [#15871](cloudflare/workers-sdk#15871) [`6a4b0fe`](cloudflare/workers-sdk@6a4b0fe) Thanks [@<!---->tw4](https://github.com/tw4)! - Retry transient API failures in `wrangler workflows instances list` and `wrangler workflows instances describe` Previously, a single temporary 5xx response or dropped connection made these read-only commands …[full notes](https://github.com/cloudflare/workers-sdk/releases/tag/wrangler%404.147.0) </details> --- *This PR was auto-generated by [catalog-update-action](https://github.com/brandhaug/catalog-update-action).* Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Fixes #15865.
wrangler workflows instances listandwrangler workflows instances describemade a single API request and exited with code 1 on a transient failure, such as a 503 or a dropped connection, even when the next request would have succeeded.These read-only requests now go through the existing
retryOnAPIFailurehelper: up to 3 attempts, retrying on 5xx, 429 (honouringRetry-After), request timeouts and network errors. Persistent failures still surface once the attempts are exhausted.describe: retries the instance request, and the request that resolves--id latest(the default) ingetInstanceIdFromArgs, which is the first requestdescribemakes.pause,resume,terminate,restart,send-eventanddeleteuse the same helper to resolvelatest, so that lookup is retried for them too. The mutating requests those commands make afterwards are not retried.list:fetchCursorPagedeletespagefrom theURLSearchParamsit receives, so each attempt gets its own copy. Without that, a retry would silently return page 1 for--page N.--json:retryOnAPIFailurereportsRetry-Afterwaits withlogger.info, which writes to stdout. Under--json,getRetryLoggersends these notices to stderr so stdout stays valid JSON.A retry in
listrestarts the cursor walk from the first page, becausefetchCursorPagehas no way to resume from a cursor. This is the same root cause as #15866 and is left for that issue.Manual testing
Point the CLI at a local server that returns
503(optionally withRetry-After: 0) or drops the connection on the first request and succeeds on the next, for example viaCLOUDFLARE_API_BASE_URL, then run:wrangler workflows instances list <name> --jsonwrangler workflows instances list <name> --page 2 --jsonwrangler workflows instances describe <name> --jsonEach command should succeed and print only JSON on stdout.