Merge branch 'master' into fix/rate-limit-message

# Conflicts:
#	packages/backend-common/src/reading/GithubUrlReader.ts

.changeset/eight-rice-cross.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Do not call fetch directly but rather use `fetchResponse` facility

.changeset/fair-spies-rescue.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend-module-github': patch
+---
+
+Ensure `teamReviewers` list is unique before calling API

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);
       }

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'],
       });
     });
 

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(', ') ?? '';