diff --git a/.changeset/fine-cameras-deny.md b/.changeset/fine-cameras-deny.md new file mode 100644 index 0000000000..f5493c2fa3 --- /dev/null +++ b/.changeset/fine-cameras-deny.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend-module-github': patch +--- + +Improved Octokit client creation to support retries via @octokit/plugin-retry diff --git a/plugins/scaffolder-backend-module-github/package.json b/plugins/scaffolder-backend-module-github/package.json index 982f5692f4..830f5615ff 100644 --- a/plugins/scaffolder-backend-module-github/package.json +++ b/plugins/scaffolder-backend-module-github/package.json @@ -51,6 +51,7 @@ "@backstage/plugin-scaffolder-node": "workspace:^", "@backstage/types": "workspace:^", "@octokit/core": "^5.0.0", + "@octokit/plugin-retry": "^6.0.0", "@octokit/webhooks": "^10.9.2", "libsodium-wrappers": "^0.8.0", "octokit": "^3.0.0", diff --git a/plugins/scaffolder-backend-module-github/report.api.md b/plugins/scaffolder-backend-module-github/report.api.md index 382665eea1..0e1323d08a 100644 --- a/plugins/scaffolder-backend-module-github/report.api.md +++ b/plugins/scaffolder-backend-module-github/report.api.md @@ -8,6 +8,7 @@ import { CatalogService } from '@backstage/plugin-catalog-node'; import { Config } from '@backstage/config'; import { createPullRequest } from 'octokit-plugin-create-pull-request'; import { GithubCredentialsProvider } from '@backstage/integration'; +import { LoggerService } from '@backstage/backend-plugin-api'; import { Octokit } from 'octokit'; import { OctokitOptions } from '@octokit/core/dist-types/types'; import { ScmIntegrationRegistry } from '@backstage/integration'; @@ -509,6 +510,13 @@ export const createPublishGithubPullRequestAction: ( 'v2' >; +// @public +export function getOctokitClient( + octokitOptions: OctokitOptions, + logger: LoggerService, + retryOptions?: RetryOptions, +): Octokit; + // @public export function getOctokitOptions(options: { integrations: ScmIntegrationRegistry; @@ -530,4 +538,10 @@ export function getOctokitOptions(options: { // @public const githubModule: BackendFeature; export default githubModule; + +// @public +export type RetryOptions = { + retries?: number; + retryAfter?: number; +}; ``` diff --git a/plugins/scaffolder-backend-module-github/src/index.ts b/plugins/scaffolder-backend-module-github/src/index.ts index cf0bc003cb..27e6ae60a5 100644 --- a/plugins/scaffolder-backend-module-github/src/index.ts +++ b/plugins/scaffolder-backend-module-github/src/index.ts @@ -22,4 +22,4 @@ export * from './actions'; export { githubModule as default } from './module'; -export { getOctokitOptions } from './util'; +export { getOctokitClient, getOctokitOptions, type RetryOptions } from './util'; diff --git a/plugins/scaffolder-backend-module-github/src/util.test.ts b/plugins/scaffolder-backend-module-github/src/util.test.ts new file mode 100644 index 0000000000..ed215b87d0 --- /dev/null +++ b/plugins/scaffolder-backend-module-github/src/util.test.ts @@ -0,0 +1,161 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Octokit } from 'octokit'; +import { retry } from '@octokit/plugin-retry'; +import { mockServices } from '@backstage/backend-test-utils'; +import { isRetryEnabled, getOctokitClient } from './util'; + +jest.mock('octokit', () => ({ + Octokit: Object.assign(jest.fn(), { + plugin: jest.fn(), + }), +})); + +jest.mock('@octokit/plugin-retry', () => ({ + retry: jest.fn(), +})); + +const mockOctokitInstance = { request: jest.fn() }; +const MockOctokit = Octokit as unknown as jest.MockedClass & { + plugin: jest.Mock; +}; + +describe('isRetryEnabled', () => { + it.each([ + { + description: 'returns false when retries is == 0', + options: { retries: 0 }, + expected: false, + }, + { + description: 'returns false when retries is <= 0', + options: { retries: -10 }, + expected: false, + }, + { + description: 'returns false when retryAfter is == 0', + options: { retries: 1, retryAfter: 0 }, + expected: false, + }, + { + description: 'returns false when retryAfter is <= 0', + options: { retries: 1, retryAfter: -10 }, + expected: false, + }, + { + description: 'returns true when retryOptions is undefined', + options: undefined, + expected: true, + }, + { + description: 'returns true when retries is > 0 and retryAfter is > 0', + options: { retries: 5, retryAfter: 5 }, + expected: true, + }, + ])('$description', ({ options, expected }) => { + expect(isRetryEnabled(options)).toBe(expected); + }); +}); + +describe('getOctokitClient', () => { + const logger = mockServices.logger.mock(); + const octokitOptions = { + auth: 'test-token', + baseUrl: 'https://api.github.com', + }; + + beforeEach(() => { + jest.clearAllMocks(); + MockOctokit.mockReturnValue(mockOctokitInstance as any); + MockOctokit.plugin.mockReturnValue(MockOctokit); + }); + + it('returns a plain Octokit client when retries is 0', () => { + const retryOptions = { retries: 0 }; + const client = getOctokitClient(octokitOptions, logger, retryOptions); + + expect(MockOctokit.plugin).not.toHaveBeenCalled(); + expect(MockOctokit).toHaveBeenCalledWith({ + ...octokitOptions, + log: logger, + }); + expect(client).toBe(mockOctokitInstance); + }); + + it('returns a plain Octokit client when retryAfter is 0', () => { + const retryOptions = { retries: 3, retryAfter: 0 }; + const client = getOctokitClient(octokitOptions, logger, retryOptions); + + expect(MockOctokit.plugin).not.toHaveBeenCalled(); + expect(MockOctokit).toHaveBeenCalledWith({ + ...octokitOptions, + log: logger, + }); + expect(client).toBe(mockOctokitInstance); + }); + + it('returns a retry-enabled Octokit client with default retries and delay when retryOptions is undefined', () => { + const client = getOctokitClient(octokitOptions, logger); + + expect(MockOctokit.plugin).toHaveBeenCalledWith(retry); + expect(MockOctokit).toHaveBeenCalledWith({ + ...octokitOptions, + request: { + retries: 3, + retryAfter: 1000, + }, + log: logger, + }); + expect(client).toBe(mockOctokitInstance); + }); + + it('returns a retry-enabled Octokit client with custom retries and delay', () => { + const retryOptions = { retries: 5, retryAfter: 2000 }; + const client = getOctokitClient(octokitOptions, logger, retryOptions); + + expect(MockOctokit.plugin).toHaveBeenCalledWith(retry); + expect(MockOctokit).toHaveBeenCalledWith({ + ...octokitOptions, + request: { + retries: 5, + retryAfter: 2000, + }, + log: logger, + }); + expect(client).toBe(mockOctokitInstance); + }); + + it('merges existing request options when building retry client', () => { + const optionsWithRequest = { + ...octokitOptions, + request: { timeout: 60_000 }, + }; + + const retryOptions = { retries: 3, retryAfter: 1000 }; + getOctokitClient(optionsWithRequest, logger, retryOptions); + + expect(MockOctokit).toHaveBeenCalledWith({ + ...optionsWithRequest, + request: { + timeout: 60_000, + retries: 3, + retryAfter: 1000, + }, + log: logger, + }); + }); +}); diff --git a/plugins/scaffolder-backend-module-github/src/util.ts b/plugins/scaffolder-backend-module-github/src/util.ts index 260c93993f..09a8b0b6dd 100644 --- a/plugins/scaffolder-backend-module-github/src/util.ts +++ b/plugins/scaffolder-backend-module-github/src/util.ts @@ -22,9 +22,110 @@ import { } from '@backstage/integration'; import { parseRepoUrl } from '@backstage/plugin-scaffolder-node'; import { OctokitOptions } from '@octokit/core/dist-types/types'; +import { Octokit } from 'octokit'; +import { retry } from '@octokit/plugin-retry'; +import { LoggerService } from '@backstage/backend-plugin-api'; const DEFAULT_TIMEOUT_MS = 60_000; +// By default, octokit/plugin-retry will retry 3 times with a 1 second delay +// between retries. +const DEFAULT_RETRY_ATTEMPTS = 3; +const DEFAULT_RETRY_DELAY_MS = 1_000; + +/** + * Options used to override plugin-retry defaults + * @public + */ +export type RetryOptions = { + // The number of retry attempts for failed requests + retries?: number; + // The delay in milliseconds between retry attempts + retryAfter?: number; +}; + +/** + * Helper function to determine if retries are enabled based on the provided + * options. + * + * @param retryOptions - Optional retry configuration options, including the number + * of retries and the delay between retries. + * + * @returns False if retries/retryAfter are explicitly set to 0 or less. + * Returns true otherwise. + */ +export function isRetryEnabled(retryOptions?: RetryOptions): boolean { + if (retryOptions === undefined) { + return true; + } + + if (retryOptions.retries !== undefined && retryOptions.retries <= 0) { + return false; + } + + if (retryOptions.retryAfter !== undefined && retryOptions.retryAfter <= 0) { + return false; + } + + return true; +} + +/** + * Helper for generating an authenticated Octokit client with (or without) + * retry capabilities. + * + * 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 `retryAfter` to 0 in the options. + * @public + * + * @param octokitOptions - The options for configuring the Octokit client. + * Generally provided by the `getOctokitOptions` helper. + * @param logger - LoggerService instance for logging retry attempts and + * failures. + * @param retryOptions - Optional retry configuration options, including the + * number of retries and the delay between retries. + * + * @returns An authenticated Octokit client instance based on the provided + * options. + */ +export function getOctokitClient( + octokitOptions: OctokitOptions, + logger: LoggerService, + retryOptions?: RetryOptions, +): Octokit { + // Default behavior is to enable retries, but allow callers to disable by + // explicitly setting retries or retryAfter to 0 + if (!isRetryEnabled(retryOptions)) { + return new Octokit({ + ...octokitOptions, + log: logger, + }); + } + + // Update the octokit options to include retry configuration with logging + const OctokitClient = Octokit.plugin(retry); + + // From the octokit/plugin-retry documentation, specifying these values will + // always retry regardless of the response code + const retries = retryOptions?.retries + ? retryOptions?.retries + : DEFAULT_RETRY_ATTEMPTS; + const retryAfter = retryOptions?.retryAfter + ? retryOptions?.retryAfter + : DEFAULT_RETRY_DELAY_MS; + + return new OctokitClient({ + ...octokitOptions, + request: { + ...octokitOptions.request, + retries, + retryAfter, + }, + log: logger, + }); +} + /** * Helper for generating octokit configuration options. * If no token is provided, it will attempt to get a token from the credentials provider. diff --git a/yarn.lock b/yarn.lock index 75e142f9c2..5b0ae8fc01 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6733,6 +6733,7 @@ __metadata: "@backstage/plugin-scaffolder-node-test-utils": "workspace:^" "@backstage/types": "workspace:^" "@octokit/core": "npm:^5.0.0" + "@octokit/plugin-retry": "npm:^6.0.0" "@octokit/webhooks": "npm:^10.9.2" "@types/libsodium-wrappers": "npm:^0.8.0" fs-extra: "npm:^11.2.0"