diff --git a/CHANGELOG.md b/CHANGELOG.md index 594f30d8..aca40e8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed issue with GitHub app token tracking and refreshing. [#583](https://github.com/sourcebot-dev/sourcebot/pull/583) - Fixed "The account is already associated with another user" errors with GitLab oauth provider. [#584](https://github.com/sourcebot-dev/sourcebot/pull/584) - Fixed error when viewing a generic git connection in `/settings/connections`. [#588](https://github.com/sourcebot-dev/sourcebot/pull/588) +- Fixed issue with an unbounded `Promise.allSettled(...)` when retrieving details from the GitHub API about a large number of repositories (or orgs or users). [#591](https://github.com/sourcebot-dev/sourcebot/pull/591) ## Removed - Removed built-in secret manager. [#592](https://github.com/sourcebot-dev/sourcebot/pull/592) diff --git a/packages/backend/package.json b/packages/backend/package.json index f1466c0e..8b0659d2 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -51,6 +51,7 @@ "ioredis": "^5.4.2", "lowdb": "^7.0.1", "micromatch": "^4.0.8", + "p-limit": "^7.2.0", "posthog-node": "^4.2.1", "prom-client": "^15.1.3", "simple-git": "^3.27.0", diff --git a/packages/backend/src/github.ts b/packages/backend/src/github.ts index 7d4d617f..5b8aaa5a 100644 --- a/packages/backend/src/github.ts +++ b/packages/backend/src/github.ts @@ -4,6 +4,7 @@ import { createLogger } from "@sourcebot/logger"; import { GithubConnectionConfig } from "@sourcebot/schemas/v3/github.type"; import { hasEntitlement } from "@sourcebot/shared"; import micromatch from "micromatch"; +import pLimit from "p-limit"; import { processPromiseResults, throwIfAnyFailed } from "./connectionUtils.js"; import { GithubAppManager } from "./ee/githubAppManager.js"; import { env } from "./env.js"; @@ -11,6 +12,10 @@ import { fetchWithRetry, measure } from "./utils.js"; import { getTokenFromConfig } from "@sourcebot/crypto"; export const GITHUB_CLOUD_HOSTNAME = "github.com"; + +// Limit concurrent GitHub requests to avoid hitting rate limits and overwhelming installations. +const MAX_CONCURRENT_GITHUB_QUERIES = 5; +const githubQueryLimit = pLimit(MAX_CONCURRENT_GITHUB_QUERIES); const logger = createLogger('github'); export type OctokitRepository = { @@ -194,7 +199,7 @@ export const getReposForAuthenticatedUser = async (visibility: 'all' | 'private' } const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: AbortSignal, url?: string) => { - const results = await Promise.allSettled(users.map(async (user) => { + const results = await Promise.allSettled(users.map((user) => githubQueryLimit(async () => { try { logger.debug(`Fetching repository info for user ${user}...`); @@ -243,7 +248,7 @@ const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: A } throw error; } - })); + }))); throwIfAnyFailed(results); const { validItems: repos, warnings } = processPromiseResults(results); @@ -255,7 +260,7 @@ const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: A } const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSignal, url?: string) => { - const results = await Promise.allSettled(orgs.map(async (org) => { + const results = await Promise.allSettled(orgs.map((org) => githubQueryLimit(async () => { try { logger.debug(`Fetching repository info for org ${org}...`); @@ -291,7 +296,7 @@ const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSi } throw error; } - })); + }))); throwIfAnyFailed(results); const { validItems: repos, warnings } = processPromiseResults(results); @@ -303,7 +308,7 @@ const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSi } const getRepos = async (repoList: string[], octokit: Octokit, signal: AbortSignal, url?: string) => { - const results = await Promise.allSettled(repoList.map(async (repo) => { + const results = await Promise.allSettled(repoList.map((repo) => githubQueryLimit(async () => { try { const [owner, repoName] = repo.split('/'); logger.debug(`Fetching repository info for ${repo}...`); @@ -341,7 +346,7 @@ const getRepos = async (repoList: string[], octokit: Octokit, signal: AbortSigna } throw error; } - })); + }))); throwIfAnyFailed(results); const { validItems: repos, warnings } = processPromiseResults(results); diff --git a/yarn.lock b/yarn.lock index 4afea5e7..402e6610 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7807,6 +7807,7 @@ __metadata: json-schema-to-typescript: "npm:^15.0.4" lowdb: "npm:^7.0.1" micromatch: "npm:^4.0.8" + p-limit: "npm:^7.2.0" posthog-node: "npm:^4.2.1" prom-client: "npm:^15.1.3" simple-git: "npm:^3.27.0" @@ -16463,6 +16464,15 @@ __metadata: languageName: node linkType: hard +"p-limit@npm:^7.2.0": + version: 7.2.0 + resolution: "p-limit@npm:7.2.0" + dependencies: + yocto-queue: "npm:^1.2.1" + checksum: 10c0/18e5ea305c31fdc0cf5260da7b63adfd46748cd72d4b40a31a13bd23eeece729c9308047fb5d848d7fa006d1b2fc2f36bb0d9dd173a300c38cd6dc1ed3355382 + languageName: node + linkType: hard + "p-locate@npm:^5.0.0": version: 5.0.0 resolution: "p-locate@npm:5.0.0" @@ -20836,6 +20846,13 @@ __metadata: languageName: node linkType: hard +"yocto-queue@npm:^1.2.1": + version: 1.2.1 + resolution: "yocto-queue@npm:1.2.1" + checksum: 10c0/5762caa3d0b421f4bdb7a1926b2ae2189fc6e4a14469258f183600028eb16db3e9e0306f46e8ebf5a52ff4b81a881f22637afefbef5399d6ad440824e9b27f9f + languageName: node + linkType: hard + "zod-to-json-schema@npm:^3.24.1, zod-to-json-schema@npm:^3.24.5": version: 3.24.5 resolution: "zod-to-json-schema@npm:3.24.5"