From 9a2707d4b8832c885e52d29bf45bdde9b24578f4 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Fri, 9 Oct 2026 06:17:40 +0000 Subject: [PATCH] fix: preserve probe errors and complete refresh alerts --- src/core/AnonymizedFile.ts | 5 +- src/core/Repository.ts | 11 +++-- src/core/source/GitHubBase.ts | 8 ++- src/core/source/GitHubRepository.ts | 3 +- src/core/source/GitHubStream.ts | 11 +++-- test/owner-notifications.test.js | 75 +++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 11 deletions(-) diff --git a/src/core/AnonymizedFile.ts b/src/core/AnonymizedFile.ts index da1d92f..6bdd354 100644 --- a/src/core/AnonymizedFile.ts +++ b/src/core/AnonymizedFile.ts @@ -270,7 +270,10 @@ export default class AnonymizedFile { fetchFileInfoFromPath?: (filePath: string) => Promise; }; if (typeof source.fetchFileInfoFromPath !== "function") return null; - const recovered = await source.fetchFileInfoFromPath(this.anonymizedPath); + const recovered = await source.fetchFileInfoFromPath(this.anonymizedPath).catch(error => { + void notifyOwnerAccessProblem(this.repository.owner.id, error); + throw error; + }); if (!recovered) return null; recovered.repoId = this.repository.repoId; recovered.treeGeneration = this.repository.model.treeGeneration; diff --git a/src/core/Repository.ts b/src/core/Repository.ts index 969541f..0e1ce4a 100644 --- a/src/core/Repository.ts +++ b/src/core/Repository.ts @@ -628,6 +628,14 @@ export default class Repository { /** Update the repository if a new commit exists. */ async updateIfNeeded(opt?: { force: boolean }): Promise { + try { await this.refreshIfNeeded(opt); } + catch (error) { + void notifyOwnerAccessProblem(this.owner.id, error); + throw error; + } + } + + private async refreshIfNeeded(opt?: { force: boolean }): Promise { this.assertNotArchived(); if ( this._model.options.expirationMode !== "never" && @@ -658,9 +666,6 @@ export default class Repository { repo: this.source.data.repoName, repositoryID: this.model.source.repositoryId, force: true, - }).catch(error => { - void notifyOwnerAccessProblem(this.owner.id, error); - throw error; }); // update the repository name if it has changed. Persist it diff --git a/src/core/source/GitHubBase.ts b/src/core/source/GitHubBase.ts index a56c016..c18a6e0 100644 --- a/src/core/source/GitHubBase.ts +++ b/src/core/source/GitHubBase.ts @@ -52,7 +52,9 @@ export async function classifyGitHubMissError( repo: data.repoName, }); return "commit_not_found"; - } catch { + } catch (probeError) { + if ((probeError as { status?: number; httpStatus?: number }).status !== 404 && + (probeError as { httpStatus?: number }).httpStatus !== 404) throw probeError; // Repo no longer exists at owner/repo. Try to recover via the cached // numeric GitHub id — if the repo was renamed, GET /repositories/{id} // resolves to its new full_name. See #409. @@ -87,7 +89,9 @@ export async function classifyGitHubMissError( return "repo_renamed"; } return "repo_not_found"; - } catch { + } catch (probeError) { + if ((probeError as { status?: number; httpStatus?: number }).status !== 404 && + (probeError as { httpStatus?: number }).httpStatus !== 404) throw probeError; return "repo_not_found"; } } diff --git a/src/core/source/GitHubRepository.ts b/src/core/source/GitHubRepository.ts index 9ac67d6..3ed5a9d 100644 --- a/src/core/source/GitHubRepository.ts +++ b/src/core/source/GitHubRepository.ts @@ -77,7 +77,8 @@ export class GitHubRepository { try { await oct.repos.get({ owner: this.owner, repo: this.repo }); repoExists = true; - } catch { + } catch (probeError) { + if ((probeError as { status?: number }).status !== 404) throw probeError; repoExists = false; } throw new AnonymousError( diff --git a/src/core/source/GitHubStream.ts b/src/core/source/GitHubStream.ts index 4d0b36a..0a8a532 100644 --- a/src/core/source/GitHubStream.ts +++ b/src/core/source/GitHubStream.ts @@ -412,10 +412,13 @@ export default class GitHubStream extends GitHubBase { size: data.size, }; } catch (error) { - logger.debug("fetchFileInfoFromPath miss", { - filePath, - error: serializeError(error), - }); + if ((error as { status?: number }).status !== 404) throw error; + // A missing file and lost repository access both return 404. Only + // report a normal miss after the repository probe remains readable. + const code = await classifyGitHubMissError(error, this.data); + if (code === "repo_not_found") { + throw new AnonymousError(code, { httpStatus: 404, cause: error as Error }); + } return null; } } diff --git a/test/owner-notifications.test.js b/test/owner-notifications.test.js index ca18a7b..54c94fa 100644 --- a/test/owner-notifications.test.js +++ b/test/owner-notifications.test.js @@ -247,6 +247,17 @@ describe("repository notification hooks", () => { await failsWith(() => repo.files(), error); expect(sent).to.deep.equal([[repo.owner.id, error]]); }); + for (const step of ["branches", "commit"]) it(`notifies on ${step} access errors during refresh`, async () => { + const error = Object.assign(new Error("Forbidden"), { status: 403 }); + repo.getToken = async () => "token"; + stub(github, "getRepositoryFromGitHub", async () => ({ + fullName: repo.model.source.repositoryName, model: { defaultBranch: "main" }, + branches: async () => { if (step === "branches") throw error; return [{ name: "main", commit: "new-sha" }]; }, + getCommitInfo: async () => { throw error; }, + })); + await failsWith(() => repo.updateIfNeeded({ force: true }), error); + expect(sent).to.deep.equal([[repo.owner.id, error]]); + }); it("notifies the saved owner when a repository refresh cannot read GitHub", async () => { const error = new Error("repo_not_found"); repo.getToken = async () => "token"; @@ -350,3 +361,67 @@ describe("token validation preserves upstream failures", () => { expect(await tokens.checkToken(token)).to.equal(false); }); }); + +describe("repository probes and truncated file recovery", () => { + const github = require("../src/core/GitHubUtils"); + const db = require("../src/server/database"); + const Model = require("../src/core/model/repositories/repositories.model").default; + const Files = require("../src/core/model/files/files.model").default; + const File = require("../src/core/AnonymizedFile").default; + const GitHubStream = require("../src/core/source/GitHubStream").default; + const { classifyGitHubMissError } = require("../src/core/source/GitHubBase"); + const restores = []; + const stub = (object, key, value) => { const old = object[key]; restores.push(() => object[key] = old); object[key] = value; }; + const source = { getToken: async () => "probe-test", repoId: "anon", organization: "owner", repoName: "repo", commit: "sha" }; + const missing = () => Object.assign(new Error("Not found"), { status: 404 }); + afterEach(() => { while (restores.length) restores.pop()(); }); + for (const stage of ["name", "id"]) for (const status of [undefined, 429, 503]) { + it(`propagates inconclusive ${stage} probe ${status}`, async () => { + const error = Object.assign(new Error("probe failed"), { status }); + stub(db, "isConnected", true); + stub(Model, "findOne", async () => ({ name: "owner/repo", externalId: "gh_123" })); + stub(github, "octokit", () => ({ repos: { get: async () => { throw stage === "name" ? error : missing(); } }, request: async () => { throw error; } })); + let caught; + try { await classifyGitHubMissError(missing(), source); } catch (err) { caught = err; } + expect(caught).to.equal(error); + expect(notifications.isAccessFailure(caught)).to.equal(false); + }); + } + it("preserves an inconclusive repository probe during commit refresh", async () => { + const { GitHubRepository } = require("../src/core/source/GitHubRepository"); + const error = Object.assign(new Error("probe unavailable"), { status: 503 }); + stub(github, "octokit", () => ({ repos: { + getCommit: async () => { throw missing(); }, get: async () => { throw error; }, + } })); + let caught; + try { await new GitHubRepository({ name: "owner/repo" }).getCommitInfo("sha", { accessToken: "token" }); } catch (err) { caught = err; } + expect(caught).to.equal(error); + expect(notifications.isAccessFailure(caught)).to.equal(false); + }); + for (const status of [403, 503]) it(`preserves truncated recovery error ${status} before file content is read`, async () => { + const error = Object.assign(new Error("contents lookup failed"), { status }); + stub(github, "octokit", () => ({ repos: { getContent: async () => { throw error; } } })); + stub(Files, "findOne", async () => null); + const sent = []; + stub(notifications, "notifyOwnerAccessProblem", async (owner, err) => { if (notifications.isAccessFailure(err)) sent.push(owner); }); + const file = new File({ anonymizedPath: "large/missing.txt", repository: { + owner: { id: "saved-owner" }, repoId: "anon", options: { terms: [] }, + model: { truncatedFolders: ["large"] }, source: new GitHubStream(source), + } }); + let caught; + try { await file.originalPath(); } catch (err) { caught = err; } + expect(caught).to.equal(error); + expect(sent).to.deep.equal(status === 403 ? ["saved-owner"] : []); + }); + for (const repoMissing of [false, true]) it(`distinguishes a missing truncated file from lost repository access: ${repoMissing}`, async () => { + stub(db, "isConnected", false); + stub(github, "octokit", () => ({ repos: { + getContent: async () => { throw missing(); }, + get: async () => { if (repoMissing) throw missing(); return {}; }, + } })); + let result, caught; + try { result = await new GitHubStream(source).fetchFileInfoFromPath("large/missing.txt"); } catch (err) { caught = err; } + if (repoMissing) expect(caught.message).to.equal("repo_not_found"); + else { expect(caught).to.equal(undefined); expect(result).to.equal(null); } + }); +});