From 41f978f788f8a7a87860e223dd48e9723b7a3097 Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Thu, 23 Apr 2026 18:11:46 +0100 Subject: [PATCH 1/4] fix(web): restore ServiceError boundary in `getFileSourceForRepo` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem In v4.16.14 (PR #1104, GitLab MR Review Agent), the core file-fetch logic was extracted from inside `sew()` into a standalone `getFileSourceForRepo` function so it could be called by privileged server-side callers (e.g. the review agent webhook handler) without going through the auth middleware. The extraction introduced a missing error boundary. The catch block inside `getFileSourceForRepo` handled two known git error patterns and **re-threw everything else**: ```ts // getFileSourceApi.ts — before this fix } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : String(error); if (errorMessage.includes('does not exist') || ...) return fileNotFound(...); if (errorMessage.includes('unknown revision') || ...) return unexpectedError(...); throw error; // ← propagates uncaught; sew() no longer wraps this code path } ``` Because `getFileSourceForRepo` is no longer inside `sew()`, any re-thrown exception escapes as a fatal error through the Next.js server task runner, producing the `z.onFatalException` / `z.attemptTask` stack trace seen in production on v4.16.14. A second contributing factor: the review agent was also changed in v4.16.14 to pass `ref: pr_payload.head_sha` where `ref` was previously `undefined`. If the bare clone hasn't fetched that commit yet, `git show` throws with `"unknown revision"` — which fell into the same unhandled re-throw path. Rolling back to v4.16.13 restored the original `sew()`-wrapped code path, which is why the rollback fixed the issue. ## Fix Two targeted changes in `getFileSourceApi.ts`: 1. **`throw error` → `return unexpectedError(errorMessage)`** — ensures `getFileSourceForRepo` always returns `FileSourceResponse | ServiceError` and never rejects its promise. This is the root-cause fix. 2. **`unexpectedError(...)` → `invalidGitRef(gitRef)`** for the `"unknown revision"` / `"bad revision"` / `"invalid object name"` branch — uses the semantically correct error type (already imported), which also gives callers a more actionable error when `head_sha` hasn't been fetched. ## Tests Added `getFileSourceApi.test.ts` with 19 tests covering: - **Repository validation** — `NOT_FOUND` when repo is absent from the DB; correct `findFirst` query shape. - **Input validation** — path traversal and null-byte paths → `FILE_NOT_FOUND`; refs starting with `-` → `INVALID_GIT_REF` (flag-injection guard). - **Git error handling** (the regression suite): - `"does not exist"` / `"fatal: path"` → `FILE_NOT_FOUND` - `"unknown revision"` (unfetched `head_sha`) → `INVALID_GIT_REF` - `"bad revision"` / `"invalid object name"` → `INVALID_GIT_REF` - **Unrecognised error → `UNEXPECTED_ERROR`, not a throw** — explicit regression test; before the fix, `.resolves` would fail because the promise rejected. - **Successful response** — source content, language, ref fallback chain (`ref` → `defaultBranch` → `HEAD`), correct `cwd` path. - **Language detection** — prefers `.gitattributes`; falls back to filename-based detection when the file is absent. ## Files changed | File | Change | |------|--------| | `packages/web/src/features/git/getFileSourceApi.ts` | `throw error` → `return unexpectedError(errorMessage)`; invalid-ref branch now returns `invalidGitRef(gitRef)` | | `packages/web/src/features/git/getFileSourceApi.test.ts` | New — 19 tests | --- .../src/features/git/getFileSourceApi.test.ts | 329 ++++++++++++++++++ .../web/src/features/git/getFileSourceApi.ts | 4 +- 2 files changed, 331 insertions(+), 2 deletions(-) create mode 100644 packages/web/src/features/git/getFileSourceApi.test.ts diff --git a/packages/web/src/features/git/getFileSourceApi.test.ts b/packages/web/src/features/git/getFileSourceApi.test.ts new file mode 100644 index 000000000..249b6e428 --- /dev/null +++ b/packages/web/src/features/git/getFileSourceApi.test.ts @@ -0,0 +1,329 @@ +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; + +// Mocks must be declared before imports +vi.mock('simple-git'); +vi.mock('@sourcebot/shared', () => ({ + getRepoPath: (repo: { id: number }) => ({ + path: `/mock/repos/${repo.id}`, + }), + env: { + AUTH_URL: 'https://sourcebot.example.com', + }, + createLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})); +vi.mock('@/lib/utils', () => ({ + getCodeHostBrowseFileAtBranchUrl: vi.fn().mockReturnValue('https://github.com/owner/repo/blob/main/src/index.ts'), + isServiceError: (obj: unknown): boolean => { + return obj !== null && typeof obj === 'object' && 'errorCode' in (obj as object); + }, +})); +vi.mock('@/app/(app)/browse/hooks/utils', () => ({ + getBrowsePath: vi.fn().mockReturnValue('/browse/github.com/owner/repo/blob/main/src/index.ts'), +})); +vi.mock('@/lib/gitattributes', () => ({ + parseGitAttributes: vi.fn().mockReturnValue({}), + resolveLanguageFromGitAttributes: vi.fn().mockReturnValue(undefined), +})); +vi.mock('@/lib/languageDetection', () => ({ + detectLanguageFromFilename: vi.fn().mockReturnValue('TypeScript'), +})); +// Required for module load; not exercised by getFileSourceForRepo directly +vi.mock('next/headers', () => ({ + headers: vi.fn().mockResolvedValue(new Headers()), +})); +vi.mock('@/middleware/sew', () => ({ + sew: async (fn: () => Promise | T): Promise => fn(), +})); +vi.mock('@/middleware/withAuth', () => ({ + withOptionalAuth: vi.fn(), +})); +vi.mock('@/ee/features/audit/factory', () => ({ + getAuditService: () => ({ + createAudit: vi.fn(), + }), +})); + +import { simpleGit } from 'simple-git'; +import { getFileSourceForRepo } from './getFileSourceApi'; + +const MOCK_ORG = { id: 1, name: 'test-org' } as Parameters[1]['org']; + +const MOCK_REPO = { + id: 123, + name: 'github.com/owner/repo', + orgId: 1, + defaultBranch: 'main', + webUrl: 'https://github.com/owner/repo', + external_codeHostType: 'GITHUB', + displayName: null, +}; + +describe('getFileSourceForRepo', () => { + const mockGitRaw = vi.fn(); + const mockCwd = vi.fn(); + const mockSimpleGit = simpleGit as unknown as Mock; + const mockFindFirst = vi.fn(); + + const mockPrisma = { + repo: { findFirst: mockFindFirst }, + } as Parameters[1]['prisma']; + + beforeEach(() => { + vi.clearAllMocks(); + + mockCwd.mockReturnValue({ raw: mockGitRaw }); + mockSimpleGit.mockReturnValue({ cwd: mockCwd }); + mockFindFirst.mockResolvedValue(MOCK_REPO); + + // Default: file show succeeds; .gitattributes not present + mockGitRaw.mockImplementation(async (args: string[]) => { + if (args[1]?.endsWith('.gitattributes')) { + throw new Error('does not exist in HEAD'); + } + return 'console.log("hello");'; + }); + }); + + describe('repository validation', () => { + it('returns NOT_FOUND when repo is absent from the database', async () => { + mockFindFirst.mockResolvedValue(null); + + const result = await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ errorCode: 'NOT_FOUND' }); + }); + + it('queries the database by repo name and orgId', async () => { + await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(mockFindFirst).toHaveBeenCalledWith({ + where: { name: 'github.com/owner/repo', orgId: 1 }, + }); + }); + }); + + describe('input validation', () => { + it('returns FILE_NOT_FOUND for path traversal attempts', async () => { + const result = await getFileSourceForRepo( + { path: 'src/../../etc/passwd', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' }); + }); + + it('returns FILE_NOT_FOUND for null-byte paths', async () => { + const result = await getFileSourceForRepo( + { path: 'src/\0evil', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' }); + }); + + it('returns INVALID_GIT_REF 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' }); + }); + }); + + describe('git error handling', () => { + it('returns FILE_NOT_FOUND when git reports the file does not exist', async () => { + mockGitRaw.mockRejectedValueOnce( + new Error("fatal: path 'src/missing.ts' does not exist in 'main'"), + ); + + const result = await getFileSourceForRepo( + { path: 'src/missing.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' }); + }); + + it('returns FILE_NOT_FOUND for "fatal: path" errors', async () => { + mockGitRaw.mockRejectedValueOnce(new Error('fatal: path not found')); + + const result = await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + 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 () => { + // 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( + new Error("fatal: ambiguous argument 'deadbeef': unknown revision or path not in the working tree"), + ); + + const result = await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'deadbeef' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' }); + }); + + it('returns INVALID_GIT_REF 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' }); + }); + + it('returns INVALID_GIT_REF 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' }); + }); + + it('returns UNEXPECTED_ERROR — not throw — for unrecognised git errors (regression: v4.16.14 fatal exception)', async () => { + // Before the fix, getFileSourceForRepo re-threw unknown errors. + // Outside sew(), this caused a fatal Next.js task-runner exception. + // After the fix, all errors are returned as ServiceError. + mockGitRaw.mockRejectedValueOnce(new Error('I/O error: device busy')); + + const result = await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ errorCode: 'UNEXPECTED_ERROR' }); + }); + + it('never rejects its returned promise for unrecognised git errors', async () => { + mockGitRaw.mockRejectedValueOnce(new Error('transient I/O error')); + + await expect( + getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ), + ).resolves.toMatchObject({ errorCode: 'UNEXPECTED_ERROR' }); + }); + }); + + describe('successful response', () => { + it('returns the file source with language detected from filename', async () => { + const result = await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ + source: 'console.log("hello");', + language: 'TypeScript', + path: 'src/index.ts', + repo: 'github.com/owner/repo', + }); + }); + + it('uses the provided ref for the git show command', async () => { + await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'abc123sha' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(mockGitRaw).toHaveBeenCalledWith(['show', 'abc123sha:src/index.ts']); + }); + + it('falls back to defaultBranch when ref is omitted', async () => { + await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(mockGitRaw).toHaveBeenCalledWith(['show', 'main:src/index.ts']); + }); + + it('falls back to HEAD when both ref and defaultBranch are absent', async () => { + mockFindFirst.mockResolvedValue({ ...MOCK_REPO, defaultBranch: null }); + + await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(mockGitRaw).toHaveBeenCalledWith(['show', 'HEAD:src/index.ts']); + }); + + it('uses the repo path from getRepoPath for the git working directory', async () => { + await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + // getRepoPath mock returns `/mock/repos/${repo.id}` + expect(mockCwd).toHaveBeenCalledWith('/mock/repos/123'); + }); + }); + + describe('language detection', () => { + it('uses language from .gitattributes when present', async () => { + const { resolveLanguageFromGitAttributes, parseGitAttributes } = await import('@/lib/gitattributes'); + const mockResolve = resolveLanguageFromGitAttributes as unknown as Mock; + const mockParse = parseGitAttributes as unknown as Mock; + + mockParse.mockReturnValue({ '*.ts': { linguist_language: 'TypeScript' } }); + mockResolve.mockReturnValue('TypeScript'); + + // Override default: .gitattributes call succeeds + mockGitRaw.mockImplementation(async (args: string[]) => { + if (args[1]?.endsWith('.gitattributes')) { + return 'linguist-language=TypeScript'; + } + return 'file content'; + }); + + const result = await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ language: 'TypeScript' }); + expect(mockResolve).toHaveBeenCalled(); + }); + + it('falls back to filename-based detection when .gitattributes is absent', async () => { + const { detectLanguageFromFilename } = await import('@/lib/languageDetection'); + const mockDetect = detectLanguageFromFilename as unknown as Mock; + mockDetect.mockReturnValue('TypeScript'); + + // Default beforeEach setup: .gitattributes throws → falls back to filename detection + const result = await getFileSourceForRepo( + { path: 'src/index.ts', repo: 'github.com/owner/repo' }, + { org: MOCK_ORG, prisma: mockPrisma }, + ); + + expect(result).toMatchObject({ language: 'TypeScript' }); + expect(mockDetect).toHaveBeenCalledWith('src/index.ts'); + }); + }); +}); diff --git a/packages/web/src/features/git/getFileSourceApi.ts b/packages/web/src/features/git/getFileSourceApi.ts index 2ba2b57cb..f57f61854 100644 --- a/packages/web/src/features/git/getFileSourceApi.ts +++ b/packages/web/src/features/git/getFileSourceApi.ts @@ -56,9 +56,9 @@ export const getFileSourceForRepo = async ( return fileNotFound(filePath, repoName); } if (errorMessage.includes('unknown revision') || errorMessage.includes('bad revision') || errorMessage.includes('invalid object name')) { - return unexpectedError(`Invalid git reference: ${gitRef}`); + return invalidGitRef(gitRef); } - throw error; + return unexpectedError(errorMessage); } let gitattributesContent: string | undefined; From 96d3336b4627eb92cf4c29889cf5ef2bf1bbc41d Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Thu, 23 Apr 2026 18:30:59 +0100 Subject: [PATCH 2/4] fix(web): harden getFileSourceForRepo error boundary 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 --- .../src/features/git/getFileSourceApi.test.ts | 52 +++++-- .../web/src/features/git/getFileSourceApi.ts | 127 +++++++++--------- packages/web/src/lib/serviceError.ts | 8 ++ 3 files changed, 114 insertions(+), 73 deletions(-) diff --git a/packages/web/src/features/git/getFileSourceApi.test.ts b/packages/web/src/features/git/getFileSourceApi.test.ts index 249b6e428..7fb32a148 100644 --- 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[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,10 +198,13 @@ 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( @@ -190,10 +212,13 @@ 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 "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( @@ -201,7 +226,10 @@ 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 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 index f57f61854..1b585ae6a 100644 --- 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 => { - 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 => sew(() => withOptionalAuth(async ({ org, prisma, user }) => { diff --git a/packages/web/src/lib/serviceError.ts b/packages/web/src/lib/serviceError.ts index f874cdb7b..a96969824 100644 --- 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.`, + }; +} + From 2c2f003abbb62e82d15a6bfba90f42a5cf1736e7 Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Thu, 23 Apr 2026 18:34:19 +0100 Subject: [PATCH 3/4] Add CHANGELOG entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aad57323..7d335d2f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Fixed a missing error boundary in `getFileSourceForRepo` introduced in v4.16.14: the function was extracted outside `sew()` but still re-threw unrecognised git exceptions, causing fatal Next.js task-runner errors. All error paths now return a `ServiceError`. Also tightened the error message for unresolved git refs (e.g. an unfetched `head_sha`) to distinguish them from syntactically invalid refs. [#1145](https://github.com/sourcebot-dev/sourcebot/pull/1145) + ## [4.16.14] - 2026-04-21 ### Added From 9143d8d51604dad2ac780766bf5b6f06bb3bc20e Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Thu, 23 Apr 2026 18:38:10 +0100 Subject: [PATCH 4/4] refactor(web): use sew() instead of bare try/catch in getFileSourceForRepo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The top-level try/catch added to getFileSourceForRepo silently swallowed unexpected errors with no Sentry capture or log entry. Replace it with sew(), which provides the same ServiceError conversion plus Sentry reporting and structured logging. The inner git-specific catch is preserved unchanged — sew() only handles anything that escapes it (DB errors, getRepoPath, URL builders, etc.). Update the sew mock in the test file to catch and convert exceptions, matching the real sew() behaviour. Co-Authored-By: Claude Sonnet 4.6 --- .../src/features/git/getFileSourceApi.test.ts | 13 +- .../web/src/features/git/getFileSourceApi.ts | 129 +++++++++--------- 2 files changed, 73 insertions(+), 69 deletions(-) diff --git a/packages/web/src/features/git/getFileSourceApi.test.ts b/packages/web/src/features/git/getFileSourceApi.test.ts index 7fb32a148..d45eafd63 100644 --- a/packages/web/src/features/git/getFileSourceApi.test.ts +++ b/packages/web/src/features/git/getFileSourceApi.test.ts @@ -44,7 +44,16 @@ vi.mock('next/headers', () => ({ headers: vi.fn().mockResolvedValue(new Headers()), })); vi.mock('@/middleware/sew', () => ({ - sew: async (fn: () => Promise | T): Promise => fn(), + sew: async (fn: () => Promise | T): Promise => { + try { + return await fn(); + } catch (error) { + return { + errorCode: 'UNEXPECTED_ERROR', + message: error instanceof Error ? error.message : String(error), + } as T; + } + }, })); vi.mock('@/middleware/withAuth', () => ({ withOptionalAuth: vi.fn(), @@ -117,7 +126,7 @@ describe('getFileSourceForRepo', () => { }); }); - it('returns UNEXPECTED_ERROR when the database throws (outer catch)', async () => { + it('returns UNEXPECTED_ERROR when the database throws (caught by sew)', async () => { mockFindFirst.mockRejectedValue(new Error('DB connection refused')); const result = await getFileSourceForRepo( diff --git a/packages/web/src/features/git/getFileSourceApi.ts b/packages/web/src/features/git/getFileSourceApi.ts index 1b585ae6a..df22896ab 100644 --- a/packages/web/src/features/git/getFileSourceApi.ts +++ b/packages/web/src/features/git/getFileSourceApi.ts @@ -26,84 +26,79 @@ export type FileSourceResponse = z.infer; export const getFileSourceForRepo = async ( { path: filePath, repo: repoName, ref }: FileSourceRequest, { org, prisma }: { org: Org; prisma: PrismaClient }, -): Promise => { - try { - const repo = await prisma.repo.findFirst({ - where: { name: repoName, orgId: org.id }, - }); - if (!repo) { - return notFound(`Repository "${repoName}" not found.`); - } +): Promise => sew(async () => { + 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 unresolvedGitRef(gitRef); - } - return unexpectedError(errorMessage); + 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); } - - let gitattributesContent: string | undefined; - try { - gitattributesContent = await git.raw(['show', `${gitRef}:.gitattributes`]); - } catch { - // No .gitattributes in this repo/ref, that's fine + if (errorMessage.includes('unknown revision') || errorMessage.includes('bad revision') || errorMessage.includes('invalid object name')) { + return unresolvedGitRef(gitRef); } + 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 + } - 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; - } catch (error: unknown) { - const errorMessage = error instanceof Error ? error.message : String(error); - return unexpectedError(errorMessage); - } -}; + return { + source: fileContent, + language, + path: filePath, + repo: repoName, + repoCodeHostType: repo.external_codeHostType, + repoDisplayName: repo.displayName ?? undefined, + repoExternalWebUrl: repo.webUrl ?? undefined, + webUrl, + externalWebUrl, + } satisfies FileSourceResponse; +}); export const getFileSource = async ({ path: filePath, repo: repoName, ref }: FileSourceRequest, { source }: { source?: string } = {}): Promise => sew(() => withOptionalAuth(async ({ org, prisma, user }) => { if (user) {