Skip to content

feat: add retry logic with exponential backoff for async operations#82

Merged
joshjohanning merged 8 commits into
mainfrom
add-retry-logic
Mar 31, 2026
Merged

feat: add retry logic with exponential backoff for async operations#82
joshjohanning merged 8 commits into
mainfrom
add-retry-logic

Conversation

@joshjohanning

Copy link
Copy Markdown
Owner

This pull request introduces a robust retry mechanism with exponential backoff to handle transient failures when updating Git references via the GitHub API. It also adds comprehensive tests for this new functionality and updates the package version. The most important changes are summarized below:

Copilot AI review requested due to automatic review settings March 30, 2026 20:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a reusable retry helper with exponential backoff to make GitHub API ref updates more resilient to transient failures, and expands the Jest suite to validate the new behavior. This fits into the action’s release/publish flow by reducing flakiness when updating refs via Octokit.

Changes:

  • Introduced retryWithBackoff helper and wrapped octokit.rest.git.updateRef with retries.
  • Added unit tests for the retry helper and integration-style tests covering updateRef retry behavior.
  • Bumped package version to 3.0.1 and updated the coverage badge.

Reviewed changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/index.js Adds exported retry helper and uses it when updating branch refs via GitHub API.
__tests__/index.test.js Adds focused tests for retry timing/behavior and validates updateRef retry integration.
package.json Version bump reflecting behavior change.
package-lock.json Lockfile version sync with package bump.
badges/coverage.svg Updates coverage badge output after new tests.
Comments suppressed due to low confidence (2)

tests/index.test.js:1283

  • This test switches to real timers, which will force an actual ~1s sleep due to the default backoff delay and can slow down CI/flakiness. Prefer using fake timers here as well and advancing time (as in the retryWithBackoff unit tests) so the retry behavior is deterministic and fast.
  describe('updateRef retry behavior', () => {
    test('should retry updateRef on transient failure', async () => {
      jest.useRealTimers();

      mockCore.getInput.mockImplementation(name => {

tests/index.test.js:1324

  • Using an async function as a new Promise executor (and suppressing the lint rule) is error-prone and harder to read. You can usually rewrite this as const runPromise = run(); then advance timers and await runPromise (or await expect(runPromise).resolves... depending on behavior), avoiding the custom Promise wrapper entirely.
      // eslint-disable-next-line no-async-promise-executor
      const promise = new Promise(async resolve => {
        await run();
        resolve();
      });

Comment thread src/index.js Outdated
Comment thread __tests__/index.test.js Outdated
Add isTransientError() to classify retryable errors (429, 5xx, network)
vs non-retryable (400, 401, 403, 404, 422). retryWithBackoff() now
supports a custom shouldRetry predicate and logs status/code in warnings.

The updateRef call uses a custom predicate to also retry 422 'Object
does not exist' errors caused by eventual consistency after commit
creation.
@joshjohanning joshjohanning requested a review from Copilot March 30, 2026 21:01
@joshjohanning joshjohanning review requested due to automatic review settings March 30, 2026 21:01
- Guard error.message with optional chaining to handle non-Error throws
- Add explicit 'retries exhausted' warning when all attempts fail
- Switch updateRef transient test to fake timers (0.44s vs 1.3s)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.

Comment thread src/index.js
Comment thread __tests__/index.test.js Outdated
- Clamp retries to >= 0 so negative values execute fn once instead of
  silently returning undefined
- Switch exhausted-retries test to fake timers with Promise.all pattern
  to avoid real-time sleeps and unhandled rejection issues

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.

Comment thread __tests__/index.test.js
Prevents fake timer leaks if an assertion throws before manual
jest.useRealTimers() cleanup.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/index.js Outdated
Coerce retries/baseDelay to finite numbers, falling back to defaults
(3 retries, 1000ms delay) for NaN/non-numeric values.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/index.js
Guards against non-Error throws (null/undefined) in the custom
shouldRetry predicate for updateRef.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.

Comment thread src/index.js Outdated
Comment thread src/index.js
- Move transientNetworkCodes to a module-level Set constant to avoid
  per-call allocation
- Mark retryWithBackoff options as optional in JSDoc with defaults

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated no new comments.

@joshjohanning joshjohanning merged commit 40c1b4b into main Mar 31, 2026
8 checks passed
@joshjohanning joshjohanning deleted the add-retry-logic branch March 31, 2026 01:03
@github-actions

Copy link
Copy Markdown

📦 Draft Release Created

A draft release v3.0.1 has been created for this PR.

🔗 View Draft Release

Next Steps

  • Review the release notes
  • Publish the release to make it permanent

This is an automated reminder from the publish-github-action workflow.

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