Skip to content

feat: retry transient 5xx errors in withSpinner#242

Open
bukinoshita wants to merge 1 commit intomainfrom
fix/retry-transient-5xx-43c8
Open

feat: retry transient 5xx errors in withSpinner#242
bukinoshita wants to merge 1 commit intomainfrom
fix/retry-transient-5xx-43c8

Conversation

@bukinoshita
Copy link
Copy Markdown
Member

@bukinoshita bukinoshita commented Apr 9, 2026

Summary by cubic

Add opt-in retries for transient 5xx errors in withSpinner to make read paths and idempotent template writes resilient to brief outages. Addresses BU-639 by preventing failures on internal_server_error, service_unavailable, and gateway_timeout.

  • New Features

    • withSpinner now accepts { loading, retryTransient? }; always retries rate_limit_exceeded, and when retryTransient is true, also retries transient 5xx with the same backoff/retry-after logic.
    • Default retryTransient: true for runGet and runList; optional on runWrite (enabled for templates:update and templates:publish; left off for create, delete, emails:send, emails:batch; enabled for read-only emails:receiving:attachment and contact-properties:update fetch).
    • Added isTransientError utility.
  • Migration

    • Replace withSpinner('message', ...) with withSpinner({ loading: 'message' }, ...).
    • For idempotent or read-only operations, set retryTransient: true; keep it off for non-idempotent writes.
    • No changes needed where runGet/runList are used (they now retry transient errors by default).

Written for commit da1ad40. Summary will update on new commits.

@bukinoshita
Copy link
Copy Markdown
Member Author

@cursor fix conflicts

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 13, 2026

Conflicts resolved. I rebased the branch onto the latest main, resolved a merge conflict in src/commands/emails/receiving/attachment.ts (which was refactored upstream to use runGet instead of a direct withSpinner call — the runGet path already inherits retryTransient: true), and also fixed a new direct withSpinner caller in src/commands/contact-properties/update.ts that was added to main since the original PR. All 800 tests pass, typecheck and lint are clean.

Open in Web Open in Cursor 

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>
@cursor cursor bot force-pushed the fix/retry-transient-5xx-43c8 branch from 1628223 to da1ad40 Compare April 13, 2026 22:29
@bukinoshita bukinoshita marked this pull request as ready for review April 13, 2026 23:46
@bukinoshita
Copy link
Copy Markdown
Member Author

@cubic-dev-ai can you review?

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai bot commented Apr 13, 2026

@cubic-dev-ai can you review?

@bukinoshita I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 11 files

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.

2 participants