fix: preserve probe errors and complete refresh alerts

This commit is contained in:
Thomas Durieux committed 2026-10-09 06:17:40 +00:00
1 parent 2b20abe0a5
commit 9a2707d4b8
6 files changed
+102 -11

No files matched your search

+4 -1
View File
@@ -270,7 +270,10 @@ export default class AnonymizedFile {
fetchFileInfoFromPath?: (filePath: string) => Promise<IFile | null>;
};
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;
+8 -3
View File
@@ -628,6 +628,14 @@ export default class Repository {
/** Update the repository if a new commit exists. */
async updateIfNeeded(opt?: { force: boolean }): Promise<void> {
try { await this.refreshIfNeeded(opt); }
catch (error) {
void notifyOwnerAccessProblem(this.owner.id, error);
throw error;
}
}
private async refreshIfNeeded(opt?: { force: boolean }): Promise<void> {
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
+6 -2
View File
@@ -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";
}
}
+2 -1
View File
@@ -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(
+7 -4
View File
@@ -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;
}
}
+75
View File
@@ -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); }
});
});