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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ dist-bin
dist-build
*.tgz

# e2e bundle build lock (see packages/cli/test/e2e/bundle-setup.ts)
packages/cli/.bundle-build.lock

# fossilize build cache
.node-cache

Expand Down
3 changes: 3 additions & 0 deletions packages/cli/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ dist-bin
dist-build
*.tgz

# e2e bundle build lock (see test/e2e/bundle-setup.ts)
.bundle-build.lock

# fossilize build cache
.node-cache

Expand Down
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
"@clack/prompts": "0.11.0",
"@hono/node-server": "^2.0.10",
"@mastra/client-js": "^1.26.0",
"@sentry/api": "^0.253.0",
"@sentry/api": "^0.256.0",
"@sentry/core": "10.63.0",
"@sentry/node-core": "10.63.0",
"@sentry/sqlish": "^1.0.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ List events for an issue
| `platform` | string \| null | Platform (python, javascript, etc.) |
| `dateCreated` | string | ISO 8601 creation timestamp |
| `crashFile` | string \| null | Crash file URL |
| `metadata` | object \| null | Event metadata |
| `metadata` | object | Event metadata |

**Examples:**

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ List events for a specific issue
| `platform` | string \| null | Platform (python, javascript, etc.) |
| `dateCreated` | string | ISO 8601 creation timestamp |
| `crashFile` | string \| null | Crash file URL |
| `metadata` | object \| null | Event metadata |
| `metadata` | object | Event metadata |

**Examples:**

Expand Down
9 changes: 5 additions & 4 deletions packages/cli/src/lib/api/replays.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
type ListProjectReplayRecordingSegmentsResponse,
listProjectReplayRecordingSegments,
} from "@sentry/api";
import { zListProjectReplayRecordingSegmentsResponse } from "@sentry/api/zod";
import { vListProjectReplayRecordingSegmentsResponse } from "@sentry/api/valibot";
import { safeParse } from "valibot";
import type { z } from "zod";
import {
REPLAY_LIST_FIELDS,
Expand Down Expand Up @@ -123,16 +124,16 @@ type FetchReplayRecordingSegmentsPageOptions = {
* its object boundary. The SDK invokes response validators outside its normal
* error-result path, so convert Zod failures to the CLI's API error type here.
*/
// biome-ignore lint/suspicious/useAwait: the SDK's responseValidator hook requires a Promise-returning function
async function validateReplayRecordingSegmentsResponse(
data: unknown
): Promise<void> {
const result =
await zListProjectReplayRecordingSegmentsResponse.safeParseAsync(data);
const result = safeParse(vListProjectReplayRecordingSegmentsResponse, data);
if (!result.success) {
throw new ApiError(
"Unexpected replay recording segments response",
0,
result.error.message
result.issues.map((issue) => issue.message).join(", ")
);
}
}
Expand Down
26 changes: 9 additions & 17 deletions packages/cli/src/types/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,33 +102,25 @@ export type SentryProject = Partial<SdkProjectListItem> & {
// Issue Constants

/**
* Runtime-iterable tuple of issue status values, tied to the SDK's literal
* union in both directions:
* Runtime-iterable tuple of issue status values the CLI renders.
*
* - `satisfies readonly NonNullable<SdkIssueDetail["status"]>[]` catches
* **removals/renames** in the SDK union (a tuple entry that no longer
* exists in the union fails to assign).
* - `_IssueStatusParity` below catches **additions** in the SDK union
* (an SDK status missing from our tuple makes the conditional type
* reduce to `never` instead of `true`).
*
* Together they fail typechecking on any drift, forcing the tuple and the
* SDK union to stay in sync.
* This is a deliberate superset of the SDK's `GetOrganizationIssueResponse`
* status union: it keeps `resolvedInNextRelease` and `muted`, which the
* retrieve-issue endpoint still emits and the CLI still renders (see
* STATUS_ICONS / STATUS_LABELS / STATUS_COLORS). As of @sentry/api 0.256 the
* SDK union narrowed and no longer covers those two, so the previous
* `satisfies NonNullable<SdkIssueDetail["status"]>[]` drift guard misfired on
* statuses the CLI needs to display and was removed.
*/
export const ISSUE_STATUSES = [
"resolved",
"resolvedInNextRelease",
"unresolved",
"ignored",
"muted",
] as const satisfies readonly NonNullable<SdkIssueDetail["status"]>[];
] as const;
export type IssueStatus = (typeof ISSUE_STATUSES)[number];

// Note: a reverse exhaustiveness check (SDK → ISSUE_STATUSES) is not possible here
// because GetOrganizationIssueResponses is a union of all HTTP response types, one of which
// has `status: string` (loose), making SdkIssueDetail["status"] resolve to `string`.
// The `satisfies` above catches the forward direction (invalid values in our tuple).

export const ISSUE_LEVELS = [
"fatal",
"error",
Expand Down
71 changes: 56 additions & 15 deletions packages/cli/test/e2e/bundle-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,27 @@
* Shared npm bundle build helper for e2e tests.
*
* Serializes bundle builds across parallel test files so `bundle.test.ts` and
* `library.test.ts` never run `pnpm run bundle` concurrently or delete `dist/`
* while another file's build is in flight.
* `library.test.ts` never run `pnpm run bundle` concurrently. vitest runs each
* test file in its own worker process (`pool: "forks"`), so an in-process
* promise cannot coordinate them — the lock has to live on the filesystem.
* Whichever worker wins the `mkdir` lock builds once; the rest wait for the
* bundle to appear.
*/

import { spawn } from "node:child_process";
import { existsSync, rmSync } from "node:fs";
import { existsSync, mkdirSync, rmSync } from "node:fs";
import { join } from "node:path";
import { setTimeout as sleep } from "node:timers/promises";

function noop(): void {
// Intentionally empty — absorbs async spawn errors
}

const ROOT_DIR = join(import.meta.dirname, "../..");

/** Cross-process build lock directory (kept outside `dist/`). */
const LOCK_DIR = join(ROOT_DIR, ".bundle-build.lock");

/** Bundled library entrypoint used by library-mode e2e tests. */
export const BUNDLE_INDEX_PATH = join(ROOT_DIR, "dist/index.cjs");

Expand All @@ -30,26 +37,60 @@ let buildPromise: Promise<void> | null = null;
/**
* Ensure the npm bundle exists under `dist/`, building it once if needed.
*
* @param options.clean - When true, delete `dist/` before building. Only the
* first concurrent caller's preference applies while a build is in flight.
* Safe to call concurrently from multiple test files: a filesystem lock
* ensures exactly one worker runs `pnpm run bundle` while the others wait for
* the bundle to appear.
*/
export function ensureBundleBuilt(options?: {
clean?: boolean;
}): Promise<void> {
if (!options?.clean && existsSync(BUNDLE_INDEX_PATH)) {
export function ensureBundleBuilt(): Promise<void> {
if (existsSync(BUNDLE_INDEX_PATH) && !existsSync(LOCK_DIR)) {
return Promise.resolve();
}

buildPromise ??= runBundleBuild(Boolean(options?.clean));
buildPromise ??= runBundleBuild();
return buildPromise;
}

async function runBundleBuild(clean: boolean): Promise<void> {
const distDir = join(ROOT_DIR, "dist");
if (clean && existsSync(distDir)) {
rmSync(distDir, { recursive: true, force: true });
async function runBundleBuild(): Promise<void> {
// Atomic `mkdir` acts as a cross-process lock: only one worker creates the
// directory and builds; the rest fall through to wait for the bundle.
let holdsLock = false;
try {
mkdirSync(LOCK_DIR);
holdsLock = true;
} catch {
// Another worker is building — wait for the bundle to appear.
}

if (!holdsLock) {
buildPromise = null;
await waitForBundle();
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale lock wedges bundle setup

Medium Severity

If a worker dies while holding .bundle-build.lock (vitest beforeAll kill, Ctrl+C), the directory is never removed. Later ensureBundleBuilt calls skip the early return because the lock exists, fail mkdirSync, and waitForBundle times out even when a valid dist/index.cjs is already present.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0582807. Configure here.

}

try {
await spawnBundle();
} finally {
rmSync(LOCK_DIR, { recursive: true, force: true });
}

if (!existsSync(BUNDLE_INDEX_PATH)) {
buildPromise = null;
throw new Error("Bundle not built — cannot run library/bundle tests");
}
}

async function waitForBundle(): Promise<void> {
const deadline = Date.now() + 55_000;
while (Date.now() < deadline) {
if (existsSync(BUNDLE_INDEX_PATH) && !existsSync(LOCK_DIR)) {
return;
}
await sleep(250);
}
throw new Error("Bundle not built — cannot run library/bundle tests");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiters ignore build failures

Medium Severity

waitForBundle treats “dist/index.cjs exists and the lock is gone” as success. The holder always deletes the lock in finally, including after spawnBundle throws. Because the bundle script writes index.cjs before later outputs, a mid-build failure can let waiters proceed on a partial bundle while the holder errors.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0582807. Configure here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiter timeout under builder budget

Medium Severity

waitForBundle gives up after 55s, while both e2e beforeAll hooks allow 60s for the builder. A slow but successful pnpm run bundle can finish inside the holder’s budget after waiters have already timed out, causing flaky parallel e2e failures.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0582807. Configure here.

}

async function spawnBundle(): Promise<void> {
const exitCode = await new Promise<number>((resolve) => {
let buildStderr = "";
const proc = spawn("pnpm", ["run", "bundle"], {
Expand All @@ -72,7 +113,7 @@ async function runBundleBuild(clean: boolean): Promise<void> {
});
});

if (exitCode !== 0 || !existsSync(BUNDLE_INDEX_PATH)) {
if (exitCode !== 0) {
buildPromise = null;
throw new Error("Bundle not built — cannot run library/bundle tests");
}
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/e2e/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const INK_APP_PATH = join(ROOT_DIR, "dist/ink-app.js");

describe("npm bundle", () => {
beforeAll(async () => {
await ensureBundleBuilt({ clean: true });
await ensureBundleBuilt();
}, 60_000); // Bundle can take a while

test("bundle file exists", () => {
Expand Down
14 changes: 9 additions & 5 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading