diff --git a/packages/discovery-index/scripts/validate-posthog-release.mjs b/packages/discovery-index/scripts/validate-posthog-release.mjs index 89df6c1046..5534a4eed5 100644 --- a/packages/discovery-index/scripts/validate-posthog-release.mjs +++ b/packages/discovery-index/scripts/validate-posthog-release.mjs @@ -26,6 +26,19 @@ function apiBaseUrl(value) { return (nonBlank(value) ?? DEFAULT_POSTHOG_APP_HOST).replace(/\/+$/, ""); } +// GET .../error_tracking/symbol_sets returns each symbol set's `release` as a NESTED OBJECT +// ({id, hash_id, created_at, metadata, version, project}, per PostHog's own +// ErrorTrackingRelease/ErrorTrackingSymbolSet dataclasses) -- never a flat string. Reconstructing +// "{project}@{version}" here is what actually makes this comparable to our own POSTHOG_RELEASE +// convention (the same "{release-name}@{release-version}" split posthog-cli's --release-name/ +// --release-version flags combine server-side into the release posthog-cli itself resolves). +function releaseIdentifier(release) { + if (!release || typeof release !== "object") return undefined; + const project = nonBlank(release.project); + const version = nonBlank(release.version); + return project && version ? `${project}@${version}` : undefined; +} + export function loadPostHogReleaseValidationConfig(env = process.env) { return { // The same personal API key posthog-cli's upload step uses (error-tracking write + organization read @@ -89,7 +102,7 @@ export async function validatePostHogRelease(env = process.env, fetchImpl = glob requireConfig(config); const symbolSets = await fetchSymbolSets(config, fetchImpl); - const forRelease = symbolSets.filter((set) => nonBlank(set?.release) === config.release); + const forRelease = symbolSets.filter((set) => releaseIdentifier(set?.release) === config.release); const failures = []; if (forRelease.length === 0) { diff --git a/review-enrichment/scripts/validate-posthog-release.mjs b/review-enrichment/scripts/validate-posthog-release.mjs index 9d2a0deb07..1d63f823ba 100644 --- a/review-enrichment/scripts/validate-posthog-release.mjs +++ b/review-enrichment/scripts/validate-posthog-release.mjs @@ -25,6 +25,19 @@ function apiBaseUrl(value) { return (nonBlank(value) ?? DEFAULT_POSTHOG_APP_HOST).replace(/\/+$/, ""); } +// GET .../error_tracking/symbol_sets returns each symbol set's `release` as a NESTED OBJECT +// ({id, hash_id, created_at, metadata, version, project}, per PostHog's own +// ErrorTrackingRelease/ErrorTrackingSymbolSet dataclasses) -- never a flat string. Reconstructing +// "{project}@{version}" here is what actually makes this comparable to our own POSTHOG_RELEASE +// convention (the same "{release-name}@{release-version}" split posthog-cli's --release-name/ +// --release-version flags combine server-side into the release posthog-cli itself resolves). +function releaseIdentifier(release) { + if (!release || typeof release !== "object") return undefined; + const project = nonBlank(release.project); + const version = nonBlank(release.version); + return project && version ? `${project}@${version}` : undefined; +} + export function loadPostHogReleaseValidationConfig(env = process.env) { return { // The same personal API key posthog-cli's upload step uses (error-tracking write + organization read @@ -88,7 +101,7 @@ export async function validatePostHogRelease(env = process.env, fetchImpl = glob requireConfig(config); const symbolSets = await fetchSymbolSets(config, fetchImpl); - const forRelease = symbolSets.filter((set) => nonBlank(set?.release) === config.release); + const forRelease = symbolSets.filter((set) => releaseIdentifier(set?.release) === config.release); const failures = []; if (forRelease.length === 0) { diff --git a/review-enrichment/test/posthog-release-validation.test.ts b/review-enrichment/test/posthog-release-validation.test.ts index 9569c76692..ef3813fdf4 100644 --- a/review-enrichment/test/posthog-release-validation.test.ts +++ b/review-enrichment/test/posthog-release-validation.test.ts @@ -105,7 +105,11 @@ test("validatePostHogRelease falls back to statusText when a failed response bod }); test("validatePostHogRelease accepts a bare array response body (not wrapped in {results})", async () => { - const fetchImpl = async (): Promise => response([{ release: "loopover-rees@abc123", failure_reason: null }]); + // The real error_tracking/symbol_sets API returns `release` as a NESTED OBJECT + // ({id, hash_id, created_at, metadata, version, project}), never a flat string -- these fixtures use that + // real shape throughout this file (see releaseIdentifier's own comment in the source for why). + const fetchImpl = async (): Promise => + response([{ release: { project: "loopover-rees", version: "abc123" }, failure_reason: null }]); const result = await validatePostHogRelease(validationEnv(), fetchImpl); assert.deepEqual(result, { release: "loopover-rees@abc123", symbolSetCount: 1 }); }); @@ -123,7 +127,33 @@ test("validatePostHogRelease treats a response body that's neither an array nor }); test("validatePostHogRelease fails when no symbol sets match the target release", async () => { - const fetchImpl = async (): Promise => response({ results: [{ release: "some-other-release" }] }); + const fetchImpl = async (): Promise => + response({ results: [{ release: { project: "some-other", version: "release" } }] }); + await assert.rejects( + () => validatePostHogRelease(validationEnv(), fetchImpl), + (error) => { + assert(error instanceof PostHogReleaseValidationError); + assert.deepEqual(error.failures, ["no symbol sets found for release loopover-rees@abc123"]); + return true; + }, + ); +}); + +test("validatePostHogRelease fails when a symbol set's release is null (skip_release_on_fail path)", async () => { + const fetchImpl = async (): Promise => response({ results: [{ release: null }] }); + await assert.rejects( + () => validatePostHogRelease(validationEnv(), fetchImpl), + (error) => { + assert(error instanceof PostHogReleaseValidationError); + assert.deepEqual(error.failures, ["no symbol sets found for release loopover-rees@abc123"]); + return true; + }, + ); +}); + +test("validatePostHogRelease treats a release object missing version or project as non-matching", async () => { + const fetchImpl = async (): Promise => + response({ results: [{ release: { project: "loopover-rees" } }, { release: { version: "abc123" } }] }); await assert.rejects( () => validatePostHogRelease(validationEnv(), fetchImpl), (error) => { @@ -136,7 +166,9 @@ test("validatePostHogRelease fails when no symbol sets match the target release" test("validatePostHogRelease fails when a matching symbol set recorded a failure_reason", async () => { const fetchImpl = async (): Promise => - response({ results: [{ release: "loopover-rees@abc123", failure_reason: "could not parse sourcemap" }] }); + response({ + results: [{ release: { project: "loopover-rees", version: "abc123" }, failure_reason: "could not parse sourcemap" }], + }); await assert.rejects( () => validatePostHogRelease(validationEnv(), fetchImpl), (error) => { @@ -151,9 +183,9 @@ test("validatePostHogRelease succeeds when at least one matching symbol set has const fetchImpl = async (): Promise => response({ results: [ - { release: "some-other-release", failure_reason: "unrelated" }, - { release: "loopover-rees@abc123", failure_reason: null }, - { release: "loopover-rees@abc123" }, + { release: { project: "some-other", version: "release" }, failure_reason: "unrelated" }, + { release: { project: "loopover-rees", version: "abc123" }, failure_reason: null }, + { release: { project: "loopover-rees", version: "abc123" } }, ], }); const result = await validatePostHogRelease(validationEnv(), fetchImpl); @@ -164,8 +196,8 @@ test("validatePostHogRelease counts every failed symbol set, not just the first const fetchImpl = async (): Promise => response({ results: [ - { release: "loopover-rees@abc123", failure_reason: "a" }, - { release: "loopover-rees@abc123", failure_reason: "b" }, + { release: { project: "loopover-rees", version: "abc123" }, failure_reason: "a" }, + { release: { project: "loopover-rees", version: "abc123" }, failure_reason: "b" }, ], }); await assert.rejects( diff --git a/review-enrichment/test/posthog-upload.test.ts b/review-enrichment/test/posthog-upload.test.ts index 6444014901..f1dbdcac3c 100644 --- a/review-enrichment/test/posthog-upload.test.ts +++ b/review-enrichment/test/posthog-upload.test.ts @@ -55,7 +55,9 @@ async function postHogApiServer(options: { failFirstAttempt?: boolean } = {}) { res.end(JSON.stringify({ detail: "internal error" })); return; } - res.end(JSON.stringify({ results: [{ release: "loopover-rees@abc123", failure_reason: null }] })); + // The real error_tracking/symbol_sets API returns `release` as a nested {project, version} object, not + // a flat string (see validate-posthog-release.mjs's releaseIdentifier for why). + res.end(JSON.stringify({ results: [{ release: { project: "loopover-rees", version: "abc123" }, failure_reason: null }] })); }); await new Promise((resolveListen) => server.listen(0, "127.0.0.1", resolveListen)); const address = server.address(); diff --git a/test/unit/discovery-index/validate-posthog-release.test.ts b/test/unit/discovery-index/validate-posthog-release.test.ts index 57510bc546..d9dee2424b 100644 --- a/test/unit/discovery-index/validate-posthog-release.test.ts +++ b/test/unit/discovery-index/validate-posthog-release.test.ts @@ -82,8 +82,13 @@ describe("validatePostHogRelease", () => { }); }); + // The real error_tracking/symbol_sets API returns `release` as a NESTED OBJECT + // ({id, hash_id, created_at, metadata, version, project}), never a flat string -- these fixtures use that + // real shape throughout (see releaseIdentifier's own comment in the source for why). it("accepts a bare array response body (not wrapped in {results})", async () => { - const fetchImpl = vi.fn().mockResolvedValue(jsonResponse([{ release: "loopover-discovery-index@abc", failure_reason: null }])); + const fetchImpl = vi + .fn() + .mockResolvedValue(jsonResponse([{ release: { project: "loopover-discovery-index", version: "abc" }, failure_reason: null }])); await expect(validatePostHogRelease(validEnv, fetchImpl)).resolves.toEqual({ release: "loopover-discovery-index@abc", symbolSetCount: 1 }); }); @@ -95,7 +100,23 @@ describe("validatePostHogRelease", () => { }); it("fails when no symbol sets match the target release", async () => { - const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ results: [{ release: "some-other-release" }] })); + const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ results: [{ release: { project: "some-other", version: "release" } }] })); + await expect(validatePostHogRelease(validEnv, fetchImpl)).rejects.toMatchObject({ + failures: [`no symbol sets found for release loopover-discovery-index@abc`], + }); + }); + + it("fails when a symbol set's release is null (skip_release_on_fail path)", async () => { + const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ results: [{ release: null }] })); + await expect(validatePostHogRelease(validEnv, fetchImpl)).rejects.toMatchObject({ + failures: [`no symbol sets found for release loopover-discovery-index@abc`], + }); + }); + + it("treats a release object missing version or project as non-matching", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValue(jsonResponse({ results: [{ release: { project: "loopover-discovery-index" } }, { release: { version: "abc" } }] })); await expect(validatePostHogRelease(validEnv, fetchImpl)).rejects.toMatchObject({ failures: [`no symbol sets found for release loopover-discovery-index@abc`], }); @@ -103,7 +124,9 @@ describe("validatePostHogRelease", () => { it("fails when a matching symbol set recorded a failure_reason", async () => { const fetchImpl = vi.fn().mockResolvedValue( - jsonResponse({ results: [{ release: "loopover-discovery-index@abc", failure_reason: "could not parse sourcemap" }] }), + jsonResponse({ + results: [{ release: { project: "loopover-discovery-index", version: "abc" }, failure_reason: "could not parse sourcemap" }], + }), ); await expect(validatePostHogRelease(validEnv, fetchImpl)).rejects.toMatchObject({ failures: ["1 symbol set(s) for release loopover-discovery-index@abc recorded a failure_reason"], @@ -114,9 +137,9 @@ describe("validatePostHogRelease", () => { const fetchImpl = vi.fn().mockResolvedValue( jsonResponse({ results: [ - { release: "some-other-release", failure_reason: "unrelated" }, - { release: "loopover-discovery-index@abc", failure_reason: null }, - { release: "loopover-discovery-index@abc" }, + { release: { project: "some-other", version: "release" }, failure_reason: "unrelated" }, + { release: { project: "loopover-discovery-index", version: "abc" }, failure_reason: null }, + { release: { project: "loopover-discovery-index", version: "abc" } }, ], }), ); @@ -125,7 +148,12 @@ describe("validatePostHogRelease", () => { it("counts every failed symbol set, not just the first one found", async () => { const fetchImpl = vi.fn().mockResolvedValue( - jsonResponse({ results: [{ release: "loopover-discovery-index@abc", failure_reason: "a" }, { release: "loopover-discovery-index@abc", failure_reason: "b" }] }), + jsonResponse({ + results: [ + { release: { project: "loopover-discovery-index", version: "abc" }, failure_reason: "a" }, + { release: { project: "loopover-discovery-index", version: "abc" }, failure_reason: "b" }, + ], + }), ); await expect(validatePostHogRelease(validEnv, fetchImpl)).rejects.toMatchObject({ failures: ["2 symbol set(s) for release loopover-discovery-index@abc recorded a failure_reason"],