feat: retry transient 5xx errors in withSpinner#242
Merged
Conversation
Member
Author
|
@cursor fix conflicts |
|
Conflicts resolved. I rebased the branch onto the latest   |
Add opt-in retry support for transient server-side errors (internal_server_error, service_unavailable, gateway_timeout) in the withSpinner retry loop, alongside the existing rate_limit_exceeded handling. - Extract isTransientError predicate into src/lib/is-transient-error.ts - Change withSpinner first param from string to SpinnerMessages object with optional retryTransient flag - Enable retryTransient for safe read operations (runGet, runList) and idempotent writes (template update, template publish) - Keep retryTransient disabled for non-idempotent commands (create, delete, send, batch) to avoid duplicate side effects - Update all direct withSpinner callers to use the new object signature - Add tests for transient retry behavior and isTransientError predicate Co-authored-by: Bu Kinoshita <bukinoshita@users.noreply.github.com>
1628223 to
da1ad40
Compare
Member
Author
|
@cubic-dev-ai can you review? |
Contributor
@bukinoshita I have started the AI code review. It will take a few minutes to complete. |
…xx-43c8 # Conflicts: # src/lib/spinner.ts
felipefreitag
approved these changes
May 8, 2026
felipefreitag
left a comment
Contributor
There was a problem hiding this comment.
Changed the API to use the new withRetry helper
felipefreitag
added a commit
that referenced
this pull request
May 11, 2026
main #242 extracted retry logic into withRetry. Apply withTimeout to the call before passing it into withRetry, so the per-attempt timeout still holds. Replace the brittle module-spy in the spinner test with a fake-timers test, and cover timer-cleanup regressions in with-timeout.test.ts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Add opt-in retries for transient 5xx errors in
withSpinnerto make read paths and idempotent template writes resilient to brief outages. Addresses BU-639 by preventing failures oninternal_server_error,service_unavailable, andgateway_timeout.New Features
withSpinnernow accepts{ loading, retryTransient? }; always retriesrate_limit_exceeded, and whenretryTransientis true, also retries transient 5xx with the same backoff/retry-afterlogic.retryTransient: trueforrunGetandrunList; optional onrunWrite(enabled fortemplates:updateandtemplates:publish; left off forcreate,delete,emails:send,emails:batch; enabled for read-onlyemails:receiving:attachmentandcontact-properties:updatefetch).isTransientErrorutility.Migration
withSpinner('message', ...)withwithSpinner({ loading: 'message' }, ...).retryTransient: true; keep it off for non-idempotent writes.runGet/runListare used (they now retry transient errors by default).Written for commit da1ad40. Summary will update on new commits.