Skip to content

Commit 600e552

Browse files
fix: attribute Sentry errors to the loaded source release (#13719)
Attribute optional server and browser Sentry events to their source build. Use validated build commits for Docker and source/npm artifacts, preserve explicit server release overrides, and keep cached browser bundles tied to the commit they loaded. Verify 127 focused tests, server/UI typechecks, Docker and source build stamps, all 53 CI checks, and Greptile 5/5 with no unresolved comments. Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent 2a99de8 commit 600e552

14 files changed

Lines changed: 268 additions & 10 deletions

Dockerfile

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,16 +128,17 @@ COPY --from=deps /app /app
128128
COPY . .
129129
RUN find packages/paperclip-runner/runner packages/paperclip-runner/protocol -type f -exec touch -d @0 {} + \
130130
&& touch -d @0 packages/paperclip-runner/rust-toolchain.toml
131+
# Both the browser bundle and server stamp need the source commit. Declare it
132+
# after the stable dependency layers, before either application build.
133+
ARG PAPERCLIP_BUILD_COMMIT=""
131134
RUN pnpm --filter @paperclipai/ui build
132135
RUN pnpm --filter @paperclipai/plugin-sdk build
133136
# The server build runs scripts/write-build-stamp.mjs, which stamps the built
134137
# commit into dist/build-info.json. The build context has no .git, so the
135138
# script reads PAPERCLIP_BUILD_COMMIT instead. Docker exposes an ARG to the
136-
# next RUN as an environment variable, so declare it here — in the build
137-
# stage — before the server build. The production stage below declares the
139+
# next RUN as an environment variable. The production stage below declares the
138140
# same ARG again for the runtime fallback; an ARG goes out of scope at the
139141
# end of its stage. Empty for local `docker build`, which then writes no stamp.
140-
ARG PAPERCLIP_BUILD_COMMIT=""
141142
ENV NODE_OPTIONS=--max-old-space-size=4096
142143
RUN pnpm --filter @paperclipai/server build
143144
RUN test -f server/dist/index.js || (echo "ERROR: server build output missing" && exit 1)

doc/observability.md

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,23 @@ sends, so an operator can read what the feature does before turning it on.
375375
Each Sentry integration name below is verified against the default
376376
integration list of `@sentry/node@10.71.0` and `@sentry/browser@10.71.0`.
377377

378-
**Server attribute this feature sets**
378+
**Release attribution**
379+
380+
The server sets `release` to the full source commit from its build metadata.
381+
An explicit `SENTRY_RELEASE` overrides that default. If neither is available,
382+
the server leaves the release unset.
383+
384+
The browser also sets `release`, using the full `PAPERCLIP_BUILD_COMMIT`
385+
supplied when its bundle is built, or the checkout commit for source and npm
386+
builds. The server reads its packaged build stamp when no deployment marker
387+
is present. Docker passes the same commit to both
388+
application builds. A cached browser bundle keeps its own release after a
389+
server deployment, so its errors are attributed to the code actually loaded.
390+
Browser builds without a valid full commit leave the release unset. The
391+
browser does not read a release from the current server, page URL, or session.
392+
These fields contain build identifiers; they add no tenant or user identity.
393+
394+
**Server identity**
379395

380396
- `server_name` — every server event carries the host name of the process.
381397
The `@sentry/node` client already sets this value by default when the

server/scripts/write-build-stamp.mjs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// report `service.version`, so the value tracks the true built commit.
66
//
77
// The build resolves the commit in two steps:
8-
// 1. `git rev-parse --short HEAD` in the server directory.
8+
// 1. `git rev-parse HEAD` in the server directory.
99
// 2. The `PAPERCLIP_BUILD_COMMIT` environment variable.
1010
// A Docker image build excludes `.git`, so the git lookup fails there. The
1111
// image build passes the commit in `PAPERCLIP_BUILD_COMMIT` instead, so the
@@ -44,14 +44,14 @@ export function resolveBuildCommit(gitCommit, suppliedCommit) {
4444
}
4545

4646
/**
47-
* Read the short commit SHA with `git rev-parse --short HEAD` in the server
47+
* Read the full commit SHA with `git rev-parse HEAD` in the server
4848
* directory. Return the SHA, or null on any failure.
4949
*
5050
* @returns {string | null}
5151
*/
5252
function readGitCommit() {
5353
try {
54-
const out = execFileSync("git", ["rev-parse", "--short", "HEAD"], {
54+
const out = execFileSync("git", ["rev-parse", "HEAD"], {
5555
cwd: serverDir,
5656
stdio: ["ignore", "pipe", "ignore"],
5757
})

server/src/__tests__/build-commit.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,28 @@ describe("parseBuildCommit", () => {
1515
});
1616

1717
describe("readBuildCommit", () => {
18+
it("reads the built server stamp in npm packages without a deployment marker", () => {
19+
const commit = "0123456789abcdef0123456789abcdef01234567";
20+
expect(readBuildCommit({
21+
environmentCommit: null,
22+
buildCommitPath: "/app/.paperclip-build-commit",
23+
buildInfoPath: "/app/server/dist/build-info.json",
24+
readTextFile: (path) => {
25+
if (path.endsWith(".paperclip-build-commit")) throw new Error("ENOENT");
26+
return JSON.stringify({ commit });
27+
},
28+
})).toBe(commit);
29+
});
30+
31+
it.each(["invalid json", "null", '{"commit":"short"}', '{"commit":42}'])(
32+
"fails open on an invalid server stamp: %s", (stamp) => {
33+
expect(readBuildCommit({
34+
environmentCommit: null,
35+
readTextFile: () => stamp,
36+
})).toBeNull();
37+
},
38+
);
39+
1840
it("prefers an explicit environment commit", () => {
1941
const readTextFile = vi.fn(() => "ffffffffffffffffffffffffffffffffffffffff");
2042

server/src/__tests__/sentry.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,13 @@ describe("missing @sentry/node package", () => {
268268
it("logs one warning and resolves", async () => {
269269
process.env[BACKEND_DSN_ENV] = "https://public@o0.ingest.sentry.io/1";
270270
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
271+
// Keep this failure-mode test valid when the optional real-SDK tests run.
272+
vi.doMock("../peer-version-check.js", () => ({
273+
checkExactPeerVersions: () => ({
274+
ok: false,
275+
detail: { missing: ["@sentry/node"], mismatched: [] },
276+
}),
277+
}));
271278

272279
const { sentryReady } = await importFreshSentry();
273280

@@ -537,6 +544,41 @@ describe("buildSentryInitOptions serverName", () => {
537544
});
538545
});
539546

547+
describe("buildSentryInitOptions release", () => {
548+
const commit = "0123456789abcdef0123456789abcdef01234567";
549+
const integrations = {
550+
httpIntegration: () => ({ name: "Http" }),
551+
onUnhandledRejectionIntegration: () => ({ name: "OnUnhandledRejection" }),
552+
};
553+
554+
beforeEach(() => {
555+
vi.stubEnv("SENTRY_RELEASE", "");
556+
vi.doMock("../build-commit.js", () => ({ readBuildCommit: () => commit }));
557+
});
558+
559+
afterEach(() => {
560+
vi.unstubAllEnvs();
561+
vi.doUnmock("../build-commit.js");
562+
});
563+
564+
it("uses the server build commit", async () => {
565+
const { buildSentryInitOptions } = await importFreshSentry();
566+
expect(buildSentryInitOptions("test-dsn", integrations).release).toBe(commit);
567+
});
568+
569+
it("preserves an operator's explicit release", async () => {
570+
vi.stubEnv("SENTRY_RELEASE", " custom-release ");
571+
const { buildSentryInitOptions } = await importFreshSentry();
572+
expect(buildSentryInitOptions("test-dsn", integrations).release).toBe("custom-release");
573+
});
574+
575+
it("leaves an unknown build unattributed", async () => {
576+
vi.doMock("../build-commit.js", () => ({ readBuildCommit: () => null }));
577+
const { buildSentryInitOptions } = await importFreshSentry();
578+
expect(buildSentryInitOptions("test-dsn", integrations).release).toBeUndefined();
579+
});
580+
});
581+
540582
describe("with @sentry/node mocked", () => {
541583
it("initializes the client and shares captureException / shutdownSentry with it", async () => {
542584
process.env[DSN_ENV] = "https://public@o0.ingest.sentry.io/1";
@@ -644,6 +686,22 @@ describe.skipIf(!sentryPackage)("captured event shape against the real @sentry/n
644686
Sentry.init(options);
645687
}
646688

689+
it("attaches the actual build commit to an emitted event", async () => {
690+
const commit = "0123456789abcdef0123456789abcdef01234567";
691+
vi.stubEnv("PAPERCLIP_BUILD_COMMIT", commit);
692+
vi.stubEnv("SENTRY_RELEASE", "");
693+
try {
694+
let captured: Record<string, unknown> | null = null;
695+
await initRealSentryForTest((event) => { captured = event; });
696+
sentryPackage!.captureException(new Error("build attribution check"));
697+
await sentryPackage!.flush(2000);
698+
expect(captured).toMatchObject({ release: commit });
699+
expect(captured).not.toHaveProperty("request");
700+
} finally {
701+
vi.unstubAllEnvs();
702+
}
703+
});
704+
647705
it("a server event captured after a console.error call carries no console breadcrumb", async () => {
648706
const Sentry = sentryPackage!;
649707
let captured: Record<string, unknown> | null = null;

server/src/__tests__/write-build-stamp.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,32 @@
11
import { describe, expect, it } from "vitest";
2+
import { execFileSync } from "node:child_process";
3+
import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync } from "node:fs";
4+
import { tmpdir } from "node:os";
5+
import { join } from "node:path";
26

37
import { resolveBuildCommit } from "../../scripts/write-build-stamp.mjs";
48

9+
it("packages a full source commit without a Docker build argument", () => {
10+
const root = realpathSync(mkdtempSync(join(tmpdir(), "paperclip-build-stamp-")));
11+
try {
12+
const scriptDir = join(root, "server", "scripts");
13+
mkdirSync(scriptDir, { recursive: true });
14+
const script = join(scriptDir, "write-build-stamp.mjs");
15+
copyFileSync(new URL("../../scripts/write-build-stamp.mjs", import.meta.url), script);
16+
const git = (...args: string[]) => execFileSync("git", args, { cwd: root, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }).trim();
17+
git("init", "--quiet");
18+
git("-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--allow-empty", "--no-gpg-sign", "-m", "fixture");
19+
const env = { ...process.env };
20+
delete env.PAPERCLIP_BUILD_COMMIT;
21+
execFileSync(process.execPath, [script], { cwd: root, env, stdio: "pipe" });
22+
const stamp = JSON.parse(readFileSync(join(root, "server", "dist", "build-info.json"), "utf8"));
23+
expect(stamp.commit).toBe(git("rev-parse", "HEAD"));
24+
expect(stamp.commit).toMatch(/^[0-9a-f]{40}$/);
25+
} finally {
26+
rmSync(root, { recursive: true, force: true });
27+
}
28+
});
29+
530
describe("resolveBuildCommit", () => {
631
it("prefers the git commit over the supplied environment commit", () => {
732
expect(resolveBuildCommit("aaaaaaa", "bbbbbbb")).toBe("aaaaaaa");

server/src/build-commit.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const FULL_SHA_RE = /^[0-9a-f]{40}$/i;
77
const DEFAULT_BUILD_COMMIT_PATH = fileURLToPath(
88
new URL("../../.paperclip-build-commit", import.meta.url),
99
);
10+
const DEFAULT_BUILD_INFO_PATH = fileURLToPath(new URL("./build-info.json", import.meta.url));
1011

1112
export function parseBuildCommit(value: string | null | undefined): string | null {
1213
const commit = value?.trim() ?? "";
@@ -17,6 +18,7 @@ export function readBuildCommit(
1718
opts: {
1819
environmentCommit?: string | null;
1920
buildCommitPath?: string;
21+
buildInfoPath?: string;
2022
readTextFile?: ReadTextFile;
2123
} = {},
2224
): string | null {
@@ -27,9 +29,14 @@ export function readBuildCommit(
2729
);
2830
if (environmentCommit) return environmentCommit;
2931

32+
const readTextFile = opts.readTextFile ?? ((path: string) => readFileSync(path, "utf8"));
3033
try {
31-
const readTextFile = opts.readTextFile ?? ((path: string) => readFileSync(path, "utf8"));
32-
return parseBuildCommit(readTextFile(opts.buildCommitPath ?? DEFAULT_BUILD_COMMIT_PATH));
34+
const markerCommit = parseBuildCommit(readTextFile(opts.buildCommitPath ?? DEFAULT_BUILD_COMMIT_PATH));
35+
if (markerCommit) return markerCommit;
36+
} catch { /* The deployment marker is absent in npm packages. */ }
37+
try {
38+
const stamp = JSON.parse(readTextFile(opts.buildInfoPath ?? DEFAULT_BUILD_INFO_PATH));
39+
return typeof stamp?.commit === "string" ? parseBuildCommit(stamp.commit) : null;
3340
} catch {
3441
return null;
3542
}

server/src/sentry.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
// `instrumentation.ts`.
5757

5858
import os from "node:os";
59+
import { readBuildCommit } from "./build-commit.js";
5960
import { checkExactPeerVersions } from "./peer-version-check.js";
6061
import { resolveSentryDsns } from "./sentry-dsn.js";
6162

@@ -200,12 +201,13 @@ export function shutdownSentry(): Promise<void> {
200201
*/
201202
interface SentryModuleLike {
202203
httpIntegration(options: { breadcrumbs: boolean }): { name: string };
203-
onUnhandledRejectionIntegration(options: { mode: string }): { name: string };
204+
onUnhandledRejectionIntegration(options: { mode: "strict" }): { name: string };
204205
}
205206

206207
/** The `Sentry.init` options this gate builds. */
207208
export interface SentryInitOptions {
208209
dsn: string;
210+
release?: string;
209211
skipOpenTelemetrySetup: boolean;
210212
tracesSampleRate: number;
211213
sendDefaultPii: boolean;
@@ -225,6 +227,7 @@ export function buildSentryInitOptions(
225227
): SentryInitOptions {
226228
return {
227229
dsn,
230+
release: process.env.SENTRY_RELEASE?.trim() || readBuildCommit() || undefined,
228231
skipOpenTelemetrySetup: true,
229232
tracesSampleRate: 0,
230233
sendDefaultPii: false,

ui/src/lib/sentry.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,29 @@ function resolveIntegrations(
296296
}
297297

298298
describe("buildBrowserSentryInitOptions", () => {
299+
it("keeps the loaded bundle's release when the server version changes", async () => {
300+
const commit = "0123456789abcdef0123456789abcdef01234567";
301+
vi.stubGlobal("__PAPERCLIP_BUILD_COMMIT__", commit);
302+
vi.stubEnv("PAPERCLIP_BUILD_COMMIT", "abcdef0123456789abcdef0123456789abcdef01");
303+
try {
304+
const { buildBrowserSentryInitOptions } = await importFreshSentry();
305+
expect(buildBrowserSentryInitOptions(DSN).release).toBe(commit);
306+
} finally {
307+
vi.unstubAllGlobals();
308+
vi.unstubAllEnvs();
309+
}
310+
});
311+
312+
it("does not invent a release for an unstamped bundle", async () => {
313+
vi.stubGlobal("__PAPERCLIP_BUILD_COMMIT__", null);
314+
try {
315+
const { buildBrowserSentryInitOptions } = await importFreshSentry();
316+
expect(buildBrowserSentryInitOptions(DSN).release).toBeUndefined();
317+
} finally {
318+
vi.unstubAllGlobals();
319+
}
320+
});
321+
299322
it("sets the recorded built-in privacy options", async () => {
300323
const { buildBrowserSentryInitOptions } = await importFreshSentry();
301324

@@ -367,6 +390,21 @@ describe("captured event shape against the real @sentry/browser SDK", () => {
367390
return Sentry;
368391
}
369392

393+
it("attaches the bundle release to an emitted event without page context", async () => {
394+
const commit = "0123456789abcdef0123456789abcdef01234567";
395+
vi.stubGlobal("__PAPERCLIP_BUILD_COMMIT__", commit);
396+
try {
397+
let captured: Record<string, unknown> | null = null;
398+
const Sentry = await initRealSentryForTest((event) => { captured = event; });
399+
Sentry.captureException(new Error("bundle attribution check"));
400+
await Sentry.flush(2000);
401+
expect(captured).toMatchObject({ release: commit });
402+
expect(captured).not.toHaveProperty("request");
403+
} finally {
404+
vi.unstubAllGlobals();
405+
}
406+
});
407+
370408
it("an event from a page URL that holds a test capability value carries no request URL, no query string, and no referrer", async () => {
371409
window.history.pushState({}, "", "/dashboard?token=test-capability-value");
372410
Object.defineProperty(document, "referrer", {

ui/src/lib/sentry.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,11 @@ export function captureBrowserException(error: unknown): void {
155155
export function buildBrowserSentryInitOptions(dsn: string): BrowserSentryInitOptions {
156156
return {
157157
dsn,
158+
// Use the loaded bundle's build, even when the server has since deployed.
159+
release:
160+
typeof __PAPERCLIP_BUILD_COMMIT__ === "string"
161+
? __PAPERCLIP_BUILD_COMMIT__
162+
: undefined,
158163
tracesSampleRate: 0,
159164
sendDefaultPii: false,
160165
integrations: (defaults) =>

0 commit comments

Comments
 (0)