Merge pull request #182 from backstage/copilot/add-retry-logic-http-requests

Add exponential backoff retry for transient network errors in Octokit client

Author: Rugvip

lib/createAppClient.ts

@@ -1,18 +1,31 @@
 import * as core from '@actions/core';
 import { GitHub } from '@actions/github/lib/utils';
 import { createAppAuth } from '@octokit/auth-app';
+import { withRetry, RetryOptions } from './withRetry';
+
+const RETRY_OPTIONS: RetryOptions = {
+  maxAttempts: 5,
+  initialDelayMs: 500,
+};
 
 export function createAppClient() {
   const appId = core.getInput('app-id', { required: true });
   const privateKey = core.getInput('private-key', { required: true });
   const installationId = core.getInput('installation-id', { required: true });
 
-  return new GitHub({
+  const client = new GitHub({
     authStrategy: createAppAuth,
     auth: {
       appId,
       privateKey,
       installationId,
     },
   });
+
+  // Retry transient network errors (e.g. "socket hang up") with exponential backoff.
+  client.hook.wrap('request', (request, options) =>
+    withRetry(() => request(options), RETRY_OPTIONS),
+  );
+
+  return client;
 }

lib/withRetry.test.ts

@@ -0,0 +1,72 @@
+import { describe, it, expect, jest, beforeEach, afterEach } from '@jest/globals';
+import { withRetry } from './withRetry';
+
+// Patch setTimeout so retries are instant during tests.
+const realSetTimeout = global.setTimeout;
+
+describe('withRetry', () => {
+  beforeEach(() => {
+    // Make setTimeout fire instantly to keep tests fast.
+    (global as any).setTimeout = (fn: () => void) => realSetTimeout(fn, 0);
+  });
+
+  afterEach(() => {
+    global.setTimeout = realSetTimeout;
+  });
+
+  it('returns the result on the first successful attempt', async () => {
+    const fn = jest.fn<() => Promise<string>>().mockResolvedValue('ok');
+    const result = await withRetry(fn);
+    expect(result).toBe('ok');
+    expect(fn).toHaveBeenCalledTimes(1);
+  });
+
+  it('retries on "socket hang up" and succeeds on second attempt', async () => {
+    const socketError = Object.assign(new Error('socket hang up'), {
+      code: 'ECONNRESET',
+    });
+    const fn = jest
+      .fn<() => Promise<string>>()
+      .mockRejectedValueOnce(socketError)
+      .mockResolvedValue('ok');
+
+    const result = await withRetry(fn, { maxAttempts: 3, initialDelayMs: 100 });
+    expect(result).toBe('ok');
+    expect(fn).toHaveBeenCalledTimes(2);
+  });
+
+  it('retries on ECONNRESET and succeeds on third attempt', async () => {
+    const connError = Object.assign(new Error('read ECONNRESET'), {
+      code: 'ECONNRESET',
+    });
+    const fn = jest
+      .fn<() => Promise<number>>()
+      .mockRejectedValueOnce(connError)
+      .mockRejectedValueOnce(connError)
+      .mockResolvedValue(42);
+
+    const result = await withRetry(fn, { maxAttempts: 5, initialDelayMs: 1 });
+    expect(result).toBe(42);
+    expect(fn).toHaveBeenCalledTimes(3);
+  });
+
+  it('throws immediately on non-transient errors without retrying', async () => {
+    const notFound = new Error('Not Found');
+    const fn = jest.fn<() => Promise<string>>().mockRejectedValue(notFound);
+
+    await expect(
+      withRetry(fn, { maxAttempts: 5, initialDelayMs: 1 }),
+    ).rejects.toThrow('Not Found');
+    expect(fn).toHaveBeenCalledTimes(1);
+  });
+
+  it('exhausts all attempts and re-throws the last transient error', async () => {
+    const hangUp = new Error('socket hang up');
+    const fn = jest.fn<() => Promise<string>>().mockRejectedValue(hangUp);
+
+    await expect(
+      withRetry(fn, { maxAttempts: 3, initialDelayMs: 1 }),
+    ).rejects.toThrow('socket hang up');
+    expect(fn).toHaveBeenCalledTimes(3);
+  });
+});

lib/withRetry.ts

@@ -0,0 +1,53 @@
+/**
+ * Transient error patterns that should trigger a retry.
+ * These typically indicate temporary network-level failures.
+ */
+const RETRYABLE_PATTERNS = [
+  'socket hang up',
+  'ECONNRESET',
+  'ETIMEDOUT',
+  'ECONNREFUSED',
+  'EHOSTUNREACH',
+  'ENETUNREACH',
+];
+
+export interface RetryOptions {
+  /** Maximum number of attempts including the first try (default: 5) */
+  maxAttempts?: number;
+  /** Delay before the first retry in milliseconds; doubles on each attempt (default: 500) */
+  initialDelayMs?: number;
+}
+
+function isTransientError(error: unknown): boolean {
+  if (!(error instanceof Error)) return false;
+  const msg = error.message;
+  const code = (error as NodeJS.ErrnoException).code ?? '';
+  return RETRYABLE_PATTERNS.some(p => msg.includes(p) || code.includes(p));
+}
+
+/**
+ * Retries an async function with exponential backoff on transient network
+ * errors (e.g. "socket hang up", ECONNRESET). Re-throws immediately for
+ * non-transient errors or once maxAttempts is exhausted.
+ */
+export async function withRetry<T>(
+  fn: () => Promise<T>,
+  options: RetryOptions = {},
+): Promise<T> {
+  const { maxAttempts = 5, initialDelayMs = 500 } = options;
+  for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+    try {
+      return await fn();
+    } catch (error) {
+      if (!isTransientError(error) || attempt === maxAttempts) {
+        throw error;
+      }
+      await new Promise(resolve =>
+        setTimeout(resolve, initialDelayMs * 2 ** (attempt - 1)),
+      );
+    }
+  }
+  // This line is never reached; the loop always returns or throws.
+  /* istanbul ignore next */
+  throw new Error('withRetry: unexpected end of loop');
+}