diff --git a/.changeset/eight-rice-cross.md b/.changeset/eight-rice-cross.md new file mode 100644 index 0000000000..1628276817 --- /dev/null +++ b/.changeset/eight-rice-cross.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Do not call fetch directly but rather use `fetchResponse` facility diff --git a/.changeset/fair-spies-rescue.md b/.changeset/fair-spies-rescue.md new file mode 100644 index 0000000000..d7f1ea5275 --- /dev/null +++ b/.changeset/fair-spies-rescue.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend-module-github': patch +--- + +Ensure `teamReviewers` list is unique before calling API diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 83c21b2529..edd5269649 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -105,55 +105,28 @@ export class GithubUrlReader implements UrlReader { credentials, ); - let response: Response; - try { - response = await fetch(ghUrl, { - headers: { - ...credentials?.headers, - ...(options?.etag && { 'If-None-Match': options.etag }), - ...(options?.lastModifiedAfter && { - 'If-Modified-Since': options.lastModifiedAfter.toUTCString(), - }), - Accept: 'application/vnd.github.v3.raw', - }, - // TODO(freben): The signal cast is there because pre-3.x versions of - // node-fetch have a very slightly deviating AbortSignal type signature. - // The difference does not affect us in practice however. The cast can - // be removed after we support ESM for CLI dependencies and migrate to - // version 3 of node-fetch. - // https://github.com/backstage/backstage/issues/8242 - signal: options?.signal as any, - }); - } catch (e) { - throw new Error(`Unable to read ${url}, ${e}`); - } + const response = await this.fetchResponse(ghUrl, { + headers: { + ...credentials?.headers, + ...(options?.etag && { 'If-None-Match': options.etag }), + ...(options?.lastModifiedAfter && { + 'If-Modified-Since': options.lastModifiedAfter.toUTCString(), + }), + Accept: 'application/vnd.github.v3.raw', + }, + // TODO(freben): The signal cast is there because pre-3.x versions of + // node-fetch have a very slightly deviating AbortSignal type signature. + // The difference does not affect us in practice however. The cast can + // be removed after we support ESM for CLI dependencies and migrate to + // version 3 of node-fetch. + // https://github.com/backstage/backstage/issues/8242 + signal: options?.signal as any, + }); - if (response.status === 304) { - throw new NotModifiedError(); - } - - if (response.ok) { - return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { - etag: response.headers.get('ETag') ?? undefined, - lastModifiedAt: parseLastModified( - response.headers.get('Last-Modified'), - ), - }); - } - - let message = `${url} could not be read as ${ghUrl}, ${response.status} ${response.statusText}`; - if (response.status === 404) { - throw new NotFoundError(message); - } - - // GitHub returns a 403 response with a couple of headers indicating rate - // limit status. See more in the GitHub docs: - // https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting - if (this.integration.isRateLimited(response)) { - message += ' (rate limit exceeded)'; - } - - throw new Error(message); + return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { + etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified(response.headers.get('Last-Modified')), + }); } async readTree( @@ -348,6 +321,11 @@ export class GithubUrlReader implements UrlReader { if (!response.ok) { let message = `Request failed for ${urlAsString}, ${response.status} ${response.statusText}`; + + if (response.status === 304) { + throw new NotModifiedError(); + } + if (response.status === 404) { throw new NotFoundError(message); } diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts index 23ac4bda45..c5b1038a01 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts @@ -377,7 +377,7 @@ describe('createPublishGithubPullRequestAction', () => { branchName: 'new-app', description: 'This PR is really good', reviewers: ['foobar'], - teamReviewers: ['team-foo'], + teamReviewers: ['team-foo', 'team-foo', 'team-bar'], }; mockDir.setContent({ [workspacePath]: {} }); @@ -401,7 +401,7 @@ describe('createPublishGithubPullRequestAction', () => { repo: 'myrepo', pull_number: 123, reviewers: ['foobar'], - team_reviewers: ['team-foo'], + team_reviewers: ['team-foo', 'team-bar'], }); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts index 0763d4a474..2d8dc7ce8d 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts @@ -372,7 +372,7 @@ export const createPublishGithubPullRequestAction = ( repo: pr.repo, pull_number: pr.number, reviewers, - team_reviewers: teamReviewers, + team_reviewers: teamReviewers ? [...new Set(teamReviewers)] : undefined, }); const addedUsers = result.data.requested_reviewers?.join(', ') ?? ''; const addedTeams = result.data.requested_teams?.join(', ') ?? '';