Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion packages/discovery-index/scripts/validate-posthog-release.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 14 additions & 1 deletion review-enrichment/scripts/validate-posthog-release.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
48 changes: 40 additions & 8 deletions review-enrichment/test/posthog-release-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> => 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> =>
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 });
});
Expand All @@ -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> => response({ results: [{ release: "some-other-release" }] });
const fetchImpl = async (): Promise<Response> =>
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> => 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> =>
response({ results: [{ release: { project: "loopover-rees" } }, { release: { version: "abc123" } }] });
await assert.rejects(
() => validatePostHogRelease(validationEnv(), fetchImpl),
(error) => {
Expand All @@ -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> =>
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) => {
Expand All @@ -151,9 +183,9 @@ test("validatePostHogRelease succeeds when at least one matching symbol set has
const fetchImpl = async (): Promise<Response> =>
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);
Expand All @@ -164,8 +196,8 @@ test("validatePostHogRelease counts every failed symbol set, not just the first
const fetchImpl = async (): Promise<Response> =>
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(
Expand Down
4 changes: 3 additions & 1 deletion review-enrichment/test/posthog-upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((resolveListen) => server.listen(0, "127.0.0.1", resolveListen));
const address = server.address();
Expand Down
42 changes: 35 additions & 7 deletions test/unit/discovery-index/validate-posthog-release.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});

Expand All @@ -95,15 +100,33 @@ 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`],
});
});

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"],
Expand All @@ -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" } },
],
}),
);
Expand All @@ -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"],
Expand Down