fix(web): harden getFileSourceForRepo error boundary

Gavin Williams · claude · Gavin Williams · commit 96d3336b4627 · 2026-04-23T18:30:59.000+01:00
Three fixes to getFileSourceForRepo, which was extracted outside sew()
in v4.16.14 and lost its error boundary:

- Wrap the entire function body in a top-level try/catch so exceptions
  from prisma, getRepoPath, simpleGit().cwd(), language helpers, and URL
  builders are converted to unexpectedError rather than propagating as
  fatal Next.js task-runner exceptions.

- Add unresolvedGitRef() to serviceError.ts (errorCode: INVALID_GIT_REF,
  distinct message) and use it for "unknown revision"/"bad revision"/
  "invalid object name" git errors, replacing the syntactic invalidGitRef
  message that was misleading for unfetched head_sha refs.

- Fix the simple-git vi.mock() factory in the test file to map both the
  default and named exports to the same hoisted mock fn, ensuring the SUT
  and the test body reference identical mocks. Add a test for the outer
  catch (DB throws) and tighten INVALID_GIT_REF assertions to distinguish
  syntactic from unresolved-ref errors by message content.

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/packages/web/src/features/git/getFileSourceApi.test.ts b/packages/web/src/features/git/getFileSourceApi.test.ts
@@ -1,7 +1,14 @@
 import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
 
-// Mocks must be declared before imports
-vi.mock('simple-git');
+// Hoist the mock function so it can be referenced in both the vi.mock factory
+// and the test body. The SUT imports simpleGit as a default export; the factory
+// maps both default and named exports to the same fn so both resolve identically.
+const mockSimpleGit = vi.hoisted(() => vi.fn());
+
+vi.mock('simple-git', () => ({
+    default: mockSimpleGit,
+    simpleGit: mockSimpleGit,
+}));
 vi.mock('@sourcebot/shared', () => ({
     getRepoPath: (repo: { id: number }) => ({
         path: `/mock/repos/${repo.id}`,
@@ -48,7 +55,6 @@ vi.mock('@/ee/features/audit/factory', () => ({
     }),
 }));
 
-import { simpleGit } from 'simple-git';
 import { getFileSourceForRepo } from './getFileSourceApi';
 
 const MOCK_ORG = { id: 1, name: 'test-org' } as Parameters<typeof getFileSourceForRepo>[1]['org'];
@@ -66,7 +72,6 @@ const MOCK_REPO = {
 describe('getFileSourceForRepo', () => {
     const mockGitRaw = vi.fn();
     const mockCwd = vi.fn();
-    const mockSimpleGit = simpleGit as unknown as Mock;
     const mockFindFirst = vi.fn();
 
     const mockPrisma = {
@@ -111,6 +116,17 @@ describe('getFileSourceForRepo', () => {
                 where: { name: 'github.com/owner/repo', orgId: 1 },
             });
         });
+
+        it('returns UNEXPECTED_ERROR when the database throws (outer catch)', async () => {
+            mockFindFirst.mockRejectedValue(new Error('DB connection refused'));
+
+            const result = await getFileSourceForRepo(
+                { path: 'src/index.ts', repo: 'github.com/owner/repo' },
+                { org: MOCK_ORG, prisma: mockPrisma },
+            );
+
+            expect(result).toMatchObject({ errorCode: 'UNEXPECTED_ERROR' });
+        });
     });
 
     describe('input validation', () => {
@@ -132,13 +148,16 @@ describe('getFileSourceForRepo', () => {
             expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' });
         });
 
-        it('returns INVALID_GIT_REF for refs starting with "-" (flag injection guard)', async () => {
+        it('returns INVALID_GIT_REF with a syntactic message for refs starting with "-" (flag injection guard)', async () => {
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: '--upload-pack=evil' },
                 { org: MOCK_ORG, prisma: mockPrisma },
             );
 
-            expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' });
+            expect(result).toMatchObject({
+                errorCode: 'INVALID_GIT_REF',
+                message: expect.stringContaining("cannot start with '-'"),
+            });
         });
     });
 
@@ -167,7 +186,7 @@ describe('getFileSourceForRepo', () => {
             expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' });
         });
 
-        it('returns INVALID_GIT_REF when head_sha has not been fetched on the local clone ("unknown revision")', async () => {
+        it('returns INVALID_GIT_REF with an unresolved-ref message when head_sha has not been fetched ("unknown revision")', async () => {
             // This is the scenario from the v4.16.14 regression: the review agent passes
             // pr_payload.head_sha as ref, but the bare clone hasn't fetched it yet.
             mockGitRaw.mockRejectedValueOnce(
@@ -179,29 +198,38 @@ describe('getFileSourceForRepo', () => {
                 { org: MOCK_ORG, prisma: mockPrisma },
             );
 
-            expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' });
+            expect(result).toMatchObject({
+                errorCode: 'INVALID_GIT_REF',
+                message: expect.stringContaining('could not be resolved'),
+            });
         });
 
-        it('returns INVALID_GIT_REF for "bad revision" errors', async () => {
+        it('returns INVALID_GIT_REF with an unresolved-ref message for "bad revision" errors', async () => {
             mockGitRaw.mockRejectedValueOnce(new Error('fatal: bad revision'));
 
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'nonexistent' },
                 { org: MOCK_ORG, prisma: mockPrisma },
             );
 
-            expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' });
+            expect(result).toMatchObject({
+                errorCode: 'INVALID_GIT_REF',
+                message: expect.stringContaining('could not be resolved'),
+            });
         });
 
-        it('returns INVALID_GIT_REF for "invalid object name" errors', async () => {
+        it('returns INVALID_GIT_REF with an unresolved-ref message for "invalid object name" errors', async () => {
             mockGitRaw.mockRejectedValueOnce(new Error('fatal: invalid object name HEAD'));
 
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo' },
                 { org: MOCK_ORG, prisma: mockPrisma },
             );
 
-            expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' });
+            expect(result).toMatchObject({
+                errorCode: 'INVALID_GIT_REF',
+                message: expect.stringContaining('could not be resolved'),
+            });
         });
 
         it('returns UNEXPECTED_ERROR — not throw — for unrecognised git errors (regression: v4.16.14 fatal exception)', async () => {
diff --git a/packages/web/src/features/git/getFileSourceApi.ts b/packages/web/src/features/git/getFileSourceApi.ts
@@ -3,7 +3,7 @@ import { getBrowsePath } from '@/app/(app)/browse/hooks/utils';
 import { getAuditService } from '@/ee/features/audit/factory';
 import { parseGitAttributes, resolveLanguageFromGitAttributes } from '@/lib/gitattributes';
 import { detectLanguageFromFilename } from '@/lib/languageDetection';
-import { ServiceError, notFound, fileNotFound, invalidGitRef, unexpectedError } from '@/lib/serviceError';
+import { ServiceError, notFound, fileNotFound, invalidGitRef, unresolvedGitRef, unexpectedError } from '@/lib/serviceError';
 import { getCodeHostBrowseFileAtBranchUrl } from '@/lib/utils';
 import { withOptionalAuth } from '@/middleware/withAuth';
 import { env, getRepoPath } from '@sourcebot/shared';
@@ -27,77 +27,82 @@ export const getFileSourceForRepo = async (
     { path: filePath, repo: repoName, ref }: FileSourceRequest,
     { org, prisma }: { org: Org; prisma: PrismaClient },
 ): Promise<FileSourceResponse | ServiceError> => {
-    const repo = await prisma.repo.findFirst({
-        where: { name: repoName, orgId: org.id },
-    });
-    if (!repo) {
-        return notFound(`Repository "${repoName}" not found.`);
-    }
+    try {
+        const repo = await prisma.repo.findFirst({
+            where: { name: repoName, orgId: org.id },
+        });
+        if (!repo) {
+            return notFound(`Repository "${repoName}" not found.`);
+        }
 
-    if (!isPathValid(filePath)) {
-        return fileNotFound(filePath, repoName);
-    }
+        if (!isPathValid(filePath)) {
+            return fileNotFound(filePath, repoName);
+        }
 
-    if (ref !== undefined && !isGitRefValid(ref)) {
-        return invalidGitRef(ref);
-    }
+        if (ref !== undefined && !isGitRefValid(ref)) {
+            return invalidGitRef(ref);
+        }
 
-    const { path: repoPath } = getRepoPath(repo);
-    const git = simpleGit().cwd(repoPath);
+        const { path: repoPath } = getRepoPath(repo);
+        const git = simpleGit().cwd(repoPath);
 
-    const gitRef = ref ?? repo.defaultBranch ?? 'HEAD';
+        const gitRef = ref ?? repo.defaultBranch ?? 'HEAD';
 
-    let fileContent: string;
-    try {
-        fileContent = await git.raw(['show', `${gitRef}:${filePath}`]);
-    } catch (error: unknown) {
-        const errorMessage = error instanceof Error ? error.message : String(error);
-        if (errorMessage.includes('does not exist') || errorMessage.includes('fatal: path')) {
-            return fileNotFound(filePath, repoName);
-        }
-        if (errorMessage.includes('unknown revision') || errorMessage.includes('bad revision') || errorMessage.includes('invalid object name')) {
-            return invalidGitRef(gitRef);
+        let fileContent: string;
+        try {
+            fileContent = await git.raw(['show', `${gitRef}:${filePath}`]);
+        } catch (error: unknown) {
+            const errorMessage = error instanceof Error ? error.message : String(error);
+            if (errorMessage.includes('does not exist') || errorMessage.includes('fatal: path')) {
+                return fileNotFound(filePath, repoName);
+            }
+            if (errorMessage.includes('unknown revision') || errorMessage.includes('bad revision') || errorMessage.includes('invalid object name')) {
+                return unresolvedGitRef(gitRef);
+            }
+            return unexpectedError(errorMessage);
         }
-        return unexpectedError(errorMessage);
-    }
 
-    let gitattributesContent: string | undefined;
-    try {
-        gitattributesContent = await git.raw(['show', `${gitRef}:.gitattributes`]);
-    } catch {
-        // No .gitattributes in this repo/ref, that's fine
-    }
+        let gitattributesContent: string | undefined;
+        try {
+            gitattributesContent = await git.raw(['show', `${gitRef}:.gitattributes`]);
+        } catch {
+            // No .gitattributes in this repo/ref, that's fine
+        }
 
-    const language = gitattributesContent
-        ? (resolveLanguageFromGitAttributes(filePath, parseGitAttributes(gitattributesContent)) ?? detectLanguageFromFilename(filePath))
-        : detectLanguageFromFilename(filePath);
+        const language = gitattributesContent
+            ? (resolveLanguageFromGitAttributes(filePath, parseGitAttributes(gitattributesContent)) ?? detectLanguageFromFilename(filePath))
+            : detectLanguageFromFilename(filePath);
 
-    const externalWebUrl = getCodeHostBrowseFileAtBranchUrl({
-        webUrl: repo.webUrl,
-        codeHostType: repo.external_codeHostType,
-        branchName: gitRef,
-        filePath,
-    });
+        const externalWebUrl = getCodeHostBrowseFileAtBranchUrl({
+            webUrl: repo.webUrl,
+            codeHostType: repo.external_codeHostType,
+            branchName: gitRef,
+            filePath,
+        });
 
-    const baseUrl = env.AUTH_URL;
-    const webUrl = `${baseUrl}${getBrowsePath({
-        repoName: repo.name,
-        revisionName: ref,
-        path: filePath,
-        pathType: 'blob',
-    })}`;
+        const baseUrl = env.AUTH_URL;
+        const webUrl = `${baseUrl}${getBrowsePath({
+            repoName: repo.name,
+            revisionName: ref,
+            path: filePath,
+            pathType: 'blob',
+        })}`;
 
-    return {
-        source: fileContent,
-        language,
-        path: filePath,
-        repo: repoName,
-        repoCodeHostType: repo.external_codeHostType,
-        repoDisplayName: repo.displayName ?? undefined,
-        repoExternalWebUrl: repo.webUrl ?? undefined,
-        webUrl,
-        externalWebUrl,
-    } satisfies FileSourceResponse;
+        return {
+            source: fileContent,
+            language,
+            path: filePath,
+            repo: repoName,
+            repoCodeHostType: repo.external_codeHostType,
+            repoDisplayName: repo.displayName ?? undefined,
+            repoExternalWebUrl: repo.webUrl ?? undefined,
+            webUrl,
+            externalWebUrl,
+        } satisfies FileSourceResponse;
+    } catch (error: unknown) {
+        const errorMessage = error instanceof Error ? error.message : String(error);
+        return unexpectedError(errorMessage);
+    }
 };
 
 export const getFileSource = async ({ path: filePath, repo: repoName, ref }: FileSourceRequest, { source }: { source?: string } = {}): Promise<FileSourceResponse | ServiceError> => sew(() => withOptionalAuth(async ({ org, prisma, user }) => {
diff --git a/packages/web/src/lib/serviceError.ts b/packages/web/src/lib/serviceError.ts
@@ -109,3 +109,11 @@ export const invalidGitRef = (ref: string): ServiceError => {
     };
 }
 
+export const unresolvedGitRef = (ref: string): ServiceError => {
+    return {
+        statusCode: StatusCodes.BAD_REQUEST,
+        errorCode: ErrorCode.INVALID_GIT_REF,
+        message: `Git reference "${ref}" could not be resolved.`,
+    };
+}
+
