Skip to content

feat: bounded retry for transient API failures#23

Merged
c-vigo merged 31 commits intomainfrom
feature/20-bounded-retry-transient-api-failures
Mar 24, 2026
Merged

feat: bounded retry for transient API failures#23
c-vigo merged 31 commits intomainfrom
feature/20-bounded-retry-transient-api-failures

Conversation

@c-vigo
Copy link
Copy Markdown
Contributor

@c-vigo c-vigo commented Mar 24, 2026

Description

Adds bounded retry with exponential backoff for transient GitHub API failures. When the API returns transient errors (404, 5xx, 429, or 403 with rate-limit/abuse message), the action can automatically retry with configurable attempts instead of failing immediately.

Key additions:

  • src/retry.tsisTransientError, classifyError, calculateDelay, and withRetry utilities
  • MAX_ATTEMPTS env var — controls retry count (default 1 = no retries for backward compatibility)
  • All GitHub API calls in commitViaAPI (getBranchInfo, getCommit, createTree, createCommit, updateBranch) wrapped in withRetry
  • Exponential backoff with jitter to avoid thundering herd

Related Issue(s)

Closes #20

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Test updates

Changes Made

  • Added src/retry.ts with RetryConfig, isTransientError, classifyError, calculateDelay, and withRetry
  • Extended CommitOptions with maxAttempts, logger, baseDelayMs, maxDelayMs
  • Wrapped getBranchInfo, getCommit, createTree, createCommit, updateBranch in withRetry
  • Added MAX_ATTEMPTS env parsing in commit-runner.ts (invalid values clamped to 1)
  • Unit tests for retry logic, commit retry behavior, and MAX_ATTEMPTS parsing
  • README documentation for MAX_ATTEMPTS and CommitOptions retry options
  • Rebuilt dist artifacts

Testing

  • Tests pass locally (npm test)
  • Manual testing performed (describe below)

Manual Testing Details

Unit tests cover: isTransientError (404, 5xx, 429, 403 rate-limit message), classifyError, calculateDelay (backoff, jitter, max cap), withRetry (success, exhaust, non-transient no-retry, logger), commit retry behavior (maxAttempts 1/2/3), and MAX_ATTEMPTS env parsing.

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly (README.md, CONTRIBUTE.md, etc.)
  • I have updated the CHANGELOG.md in the [Unreleased] section
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Additional Notes

  • Default maxAttempts: 1 preserves existing behavior (no retries) so this is backward compatible
  • Transient errors: 404, 5xx, 429, and 403 when the error message indicates rate limit / abuse (Octokit RequestError exposes this in the message string)
  • CHANGELOG.md should be updated before or after merge (as noted in COMMIT_PLAN.md)

c-vigo and others added 27 commits March 13, 2026 09:36
## Description

This pull request optimizes multi-file commits by using the GitHub `createTree` API's inline `content` field for text files so blobs are created server-side, instead of issuing one `createBlob` request per file. Binary files (NUL byte in the first 8 KiB) still use `createBlob` with base64 encoding. Large change sets are split into chained `createTree` calls (100 entries per request) to stay within payload limits.

This reduces REST traffic for large commits (for example, on the order of five calls plus ref/commit steps for many text files, instead of one blob per file) and is easier on GitHub App secondary rate limits. Commits remain API-created and verified the same way as before; authentication is unchanged.

## Related Issue(s)

Closes #19

## Type of Change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [x] Documentation update
- [ ] Refactoring (no functional changes)
- [x] Test updates

## Changes Made

- Added `isBinaryFile`, `getFileMode`, and exported `TREE_ENTRY_CHUNK_SIZE`; refactored `createTree` to use inline `content` for text, `createBlob` for binary, and chunked tree creation when there are more than 100 entries.
- Binary blob creation is sequential (not concurrent) to avoid secondary rate-limit bursts.
- Non-UTF-8 text files: `TextDecoder({ fatal: true })` with `createBlob` fallback to avoid silent data corruption.
- `isBinaryFile`: uses `bytesRead` and scans only `buf.subarray(0, bytesRead)` to avoid false positives from zero-filled buffer tails.
- Extended Jest setup and `commit.test.ts` for inline trees, binary/mixed paths, chunking, empty `filePaths` handling, and new helpers; clarified fs mock comment for binary detection.
- Documented behavior under `[Unreleased]` in `CHANGELOG.md` (Added / Changed / Fixed). Updated `README.md` (features, module exports, example `uses` pin to `@main` with note that inline optimization is unreleased until next tag).
- Regenerated `dist/` via `npm run build` and `npm run bundle`.

## Testing

- [x] Tests pass locally (`npm test`)
- [ ] Manual testing performed (describe below)

### Manual Testing Details

Not run against the live GitHub API in this environment; behavior is covered by unit tests with mocked Octokit.

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have updated the documentation accordingly (README.md, CONTRIBUTE.md, etc.)
- [x] I have updated the CHANGELOG.md in the `[Unreleased]` section
- [x] My changes generate no new warnings or errors
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published

## Additional Notes

- Branch was renamed from `bugfix/19-createtree-api-rate-limit` to `feature/19-createtree-api-rate-limit`; issue #19 was recategorized as an enhancement with an updated title.
- Commits are intentionally atomic: tests (red), feature, docs, dist rebuild; plus Copilot follow-ups (empty createTree test, fs mock comment, README pin to `@main`, dist).
- Payload-size-aware chunking (large inline content) is tracked in #22.
Prepare tests for isTransientError, classifyError, calculateDelay,
and withRetry. TDD Red phase.
- Add isTransientError for 404, 5xx, 429, 403 rate-limit
- Add classifyError for human-readable logging
- Add calculateDelay with exponential backoff + jitter
- Add withRetry wrapper (transient-only, bounded attempts)
- TDD Green: all retry tests pass
TDD Red: tests for bounded retry on getRef, getCommit,
createTree, createCommit, updateRef. commitViaAPI does not
yet use withRetry.
- Add maxAttempts, logger, baseDelayMs, maxDelayMs to CommitOptions
- Wrap getBranchInfo, getCommit, createTree, createCommit,
  updateBranch in withRetry
- Default maxAttempts=1 (no retries) for backward compatibility
- TDD Green: all commit retry tests pass
TDD Red: tests for parsing MAX_ATTEMPTS and clamping invalid
values. Runner does not yet read MAX_ATTEMPTS.
- Read MAX_ATTEMPTS env var (default 1 = no retries)
- Clamp invalid values to 1 with info log
- Pass maxAttempts to commitViaAPI
- TDD Green: all commit-runner MAX_ATTEMPTS tests pass
- Add MAX_ATTEMPTS to Action and CLI usage
- Document transient error types and backoff behavior
- Document CommitOptions.maxAttempts and logger
@c-vigo c-vigo self-assigned this Mar 24, 2026
@c-vigo c-vigo requested a review from Copilot March 24, 2026 13:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds bounded retry with exponential backoff (and jitter) around GitHub REST calls to reduce flakiness from transient API errors, and wires it into the action via a MAX_ATTEMPTS env var / CommitOptions configuration. The PR also updates the commit implementation to reduce REST call volume by inlining text file content into createTree, adds binary detection + tree batching, and refreshes docs/tests/dist artifacts.

Changes:

  • Introduces src/retry.ts (isTransientError, classifyError, calculateDelay, withRetry) and wraps key API operations in commitViaAPI with retry.
  • Adds MAX_ATTEMPTS parsing in commit-runner.ts and passes retry config/logging into commitViaAPI.
  • Optimizes tree creation (inline content for UTF-8 text, binary detection, chained/batched createTree) and expands unit tests + docs + compiled dist.

Reviewed changes

Copilot reviewed 9 out of 32 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/retry.ts New retry utilities (classification, backoff, bounded retry wrapper).
src/commit.ts Wraps core GitHub API operations with retry; adds tree creation optimizations and helpers.
src/commit-runner.ts Parses MAX_ATTEMPTS and passes retry config/logger to commitViaAPI.
src/tests/unit/retry.test.ts Unit tests for transient classification, delay calculation, and retry behavior.
src/tests/unit/commit.test.ts Expanded tests for new tree behavior, binary detection, and commit retries.
src/tests/unit/commit-runner.test.ts Tests MAX_ATTEMPTS parsing/clamping behavior.
src/tests/setup.ts Extends mocked fs with sync fd APIs for binary detection tests.
dist/src/retry.js.map Built sourcemap for retry module.
dist/src/retry.js Built JS for retry module.
dist/src/retry.d.ts.map Built TS declaration sourcemap for retry.
dist/src/retry.d.ts Built TS declarations for retry.
dist/src/commit.js.map Updated built sourcemap for commit module changes.
dist/src/commit.js Updated built JS for commit module changes (retry + tree optimization).
dist/src/commit.d.ts.map Updated built TS declaration sourcemap for commit module.
dist/src/commit.d.ts Updated built TS declarations for commit module public surface changes.
dist/src/commit-runner.js.map Updated built sourcemap for runner changes.
dist/src/commit-runner.js Updated built JS for runner changes (MAX_ATTEMPTS parsing).
dist/src/commit-runner.d.ts.map Updated built TS declaration sourcemap for runner.
dist/src/commit-runner.d.ts Updated built TS declarations for runner docs/header.
dist/src/tests/unit/retry.test.js.map Built sourcemap for retry unit tests.
dist/src/tests/unit/retry.test.js Built JS for retry unit tests.
dist/src/tests/unit/retry.test.d.ts.map Built TS declaration sourcemap for retry unit tests.
dist/src/tests/unit/retry.test.d.ts Built TS declarations for retry unit tests.
dist/src/tests/unit/commit.test.js Updated built JS for commit unit tests.
dist/src/tests/unit/commit-runner.test.js.map Updated built sourcemap for runner unit tests.
dist/src/tests/unit/commit-runner.test.js Updated built JS for runner unit tests.
dist/src/tests/setup.js.map Updated built sourcemap for test setup changes.
dist/src/tests/setup.js Updated built JS for test setup changes.
dist/index.js Updated bundled action output including retry + runner + commit changes.
README.md Documents MAX_ATTEMPTS and new module exports; updates usage example.
CHANGELOG.md Adds [Unreleased] entries describing new functionality/behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/retry.ts
Comment thread src/retry.ts
Comment thread src/retry.ts
Comment thread src/commit.ts
Comment thread src/commit.ts
Comment thread README.md
c-vigo added 4 commits March 24, 2026 13:59
- Require typeof status === 'number' in hasStatus type guard
- Clamp maxAttempts to >= 1 (and non-finite to 1) in withRetry
- Add tests for non-numeric status and zero/negative maxAttempts
Delegate base64 blob creation to createBlob for consistency with binary path.
Use @v0.1.5 in the primary workflow example; unreleased features remain documented in the note below.
@c-vigo c-vigo merged commit 7eaaa9d into main Mar 24, 2026
2 checks passed
c-vigo added a commit that referenced this pull request Mar 24, 2026
Adds missing `[Unreleased]` CHANGELOG entries for PR #23 (bounded retry for transient API failures) and the Copilot review follow-ups:

- **Added:** Bounded retry (MAX_ATTEMPTS, CommitOptions, retry module) — issue #20
- **Changed:** README example pinned to @v0.1.5
- **Fixed:** withRetry normalization, hasStatus type guard, createBlob fallback consistency
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.

[FEATURE] Bounded retry for transient GitHub API failures (default: single attempt)

2 participants