From 9032ec7f1e0cd2306ce80f84fbe7966890ce9dfb Mon Sep 17 00:00:00 2001 From: Jan Michael Ong Date: Wed, 22 Apr 2026 09:53:01 -0700 Subject: [PATCH] chore: integrate copilot / awanlin feedback Signed-off-by: Jan Michael Ong --- .../src/util.test.ts | 16 ++++++------ .../src/util.ts | 25 ++++++++++--------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/plugins/scaffolder-backend-module-github/src/util.test.ts b/plugins/scaffolder-backend-module-github/src/util.test.ts index 9cb3035f21..9bbaceee8a 100644 --- a/plugins/scaffolder-backend-module-github/src/util.test.ts +++ b/plugins/scaffolder-backend-module-github/src/util.test.ts @@ -1,5 +1,5 @@ /* - * Copyright 2025 The Backstage Authors + * Copyright 2026 The Backstage Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,15 +39,15 @@ describe('isRetryEnabled', () => { expect(isRetryEnabled()).toBe(true); }); - it('returns true when retries is positive and retryDelay is undefined', () => { + it('returns true when retries is positive and retryAfter is undefined', () => { expect(isRetryEnabled(3)).toBe(true); }); - it('returns true when retries is undefined and retryDelay is positive', () => { + it('returns true when retries is undefined and retryAfter is positive', () => { expect(isRetryEnabled(undefined, 1000)).toBe(true); }); - it('returns true when both retries and retryDelay are positive', () => { + it('returns true when both retries and retryAfter are positive', () => { expect(isRetryEnabled(5, 2000)).toBe(true); }); @@ -55,15 +55,15 @@ describe('isRetryEnabled', () => { expect(isRetryEnabled(0)).toBe(false); }); - it('returns false when retryDelay is 0', () => { + it('returns false when retryAfter is 0', () => { expect(isRetryEnabled(undefined, 0)).toBe(false); }); - it('returns false when retries is 0 even if retryDelay is positive', () => { + it('returns false when retries is 0 even if retryAfter is positive', () => { expect(isRetryEnabled(0, 1000)).toBe(false); }); - it('returns false when retryDelay is 0 even if retries is positive', () => { + it('returns false when retryAfter is 0 even if retries is positive', () => { expect(isRetryEnabled(3, 0)).toBe(false); }); }); @@ -92,7 +92,7 @@ describe('getOctokitClient', () => { expect(client).toBe(mockOctokitInstance); }); - it('returns a plain Octokit client when retryDelay is 0', () => { + it('returns a plain Octokit client when retryAfter is 0', () => { const client = getOctokitClient(octokitOptions, logger, 3, 0); expect(MockOctokit.plugin).not.toHaveBeenCalled(); diff --git a/plugins/scaffolder-backend-module-github/src/util.ts b/plugins/scaffolder-backend-module-github/src/util.ts index 0d50b7c797..aa0bea521d 100644 --- a/plugins/scaffolder-backend-module-github/src/util.ts +++ b/plugins/scaffolder-backend-module-github/src/util.ts @@ -38,21 +38,22 @@ const DEFAULT_RETRY_DELAY_MS = 1000; * options. * * Retries are enabled by default, but can be disabled by setting either `retries` - * or `retryDelay` to 0. + * or `retryAfter` to 0. + * @public * * @param retries - The number of retry attempts for failed requests. Default is 3. * Setting to 0 will disable retries. - * @param retryDelay - The delay in milliseconds between retry attempts. + * @param retryAfter - The delay in milliseconds between retry attempts. * Default is 1000ms. Setting to 0 will disable retries. * * @returns A boolean indicating whether retries are enabled or not. */ -export function isRetryEnabled(retries?: number, retryDelay?: number): boolean { +export function isRetryEnabled(retries?: number, retryAfter?: number): boolean { if (retries === 0) { return false; } - if (retryDelay === 0) { + if (retryAfter === 0) { return false; } @@ -65,7 +66,7 @@ export function isRetryEnabled(retries?: number, retryDelay?: number): boolean { * * If retries are enabled (default), the client will retry failed requests up * to the specified number of retries and delay. - * To disable retries, set either `retries` or `retryDelay` to 0 in the options. + * To disable retries, set either `retries` or `retryAfter` to 0 in the options. * * @param octokitOptions - The options for configuring the Octokit client. * Generally provided by the `getOctokitOptions` helper. @@ -73,7 +74,7 @@ export function isRetryEnabled(retries?: number, retryDelay?: number): boolean { * failures. * @param retries - The number of retry attempts for failed requests. * Default is 3. Setting to 0 will disable retries. - * @param retryDelay - The delay in milliseconds between retry attempts. + * @param retryAfter - The delay in milliseconds between retry attempts. * Default is 1000ms. Setting to 0 will disable retries. * @returns An authenticated Octokit client instance based on the provided * options. @@ -82,11 +83,11 @@ export function getOctokitClient( octokitOptions: OctokitOptions, logger: LoggerService, retries: number = DEFAULT_RETRY_ATTEMPTS, - retryDelay: number = DEFAULT_RETRY_DELAY_MS, + retryAfter: number = DEFAULT_RETRY_DELAY_MS, ): Octokit { // Default behavior is to enable retries, but allow callers to disable by - // explicitly setting retries or retryDelay to 0 - if (!isRetryEnabled(retries, retryDelay)) { + // explicitly setting retries or retryAfter to 0 + if (!isRetryEnabled(retries, retryAfter)) { return new Octokit({ ...octokitOptions, log: logger, @@ -94,13 +95,13 @@ export function getOctokitClient( } // Update the octokit options to include retry configuration with logging - const MyOctokit = Octokit.plugin(retry); - return new MyOctokit({ + const OctokitClient = Octokit.plugin(retry); + return new OctokitClient({ ...octokitOptions, request: { ...octokitOptions.request, retries, - retryAfter: retryDelay, + retryAfter, }, log: logger, });