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
32 changes: 20 additions & 12 deletions src/lib/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ const LIBRARY_EXCLUDED_INTEGRATIONS = new Set([
...EXCLUDED_INTEGRATIONS,
"OnUncaughtException", // process.on('uncaughtException')
"OnUnhandledRejection", // process.on('unhandledRejection')
"ProcessSession", // process.on('beforeExit')
"ProcessSession", // process.on('beforeExit') — anonymous handler, no cleanup
"Http", // diagnostics_channel + trace headers
"NodeFetch", // diagnostics_channel + trace headers
"FunctionToString", // wraps Function.prototype.toString
Expand Down Expand Up @@ -355,6 +355,13 @@ export function initSentry(
const libraryMode = options?.libraryMode ?? false;
const environment = getEnv().NODE_ENV ?? "development";

// Close the previous client to clean up its internal timers and beforeExit
// handlers (client report flusher interval, log flush listener). Without
// this, re-initializing the SDK (e.g., in tests) leaks setInterval handles
// that keep the event loop alive and prevent the process from exiting.
// close(0) removes listeners synchronously; we don't need to await the flush.
Sentry.getClient()?.close(0);

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.

Global client closed during reinitialization

Medium Severity

initSentry now unconditionally calls Sentry.getClient()?.close(0) before every init. In libraryMode, concurrent SDK invocations can close another in-flight invocation’s client, which can drop telemetry and invalidate active spans. It can also close a pre-existing process-wide Sentry client not created by this module.

Fix in Cursor Fix in Web


const client = Sentry.init({
dsn: SENTRY_CLI_DSN,
enabled,
Expand All @@ -374,10 +381,12 @@ export function initSentry(
},
environment,
// Enable Sentry structured logs for non-exception telemetry (e.g., unexpected input warnings).
// Disabled in library mode — logs use timers/beforeExit that pollute the host process.
enableLogs: !libraryMode,
// Disable client reports in library mode — they use timers/beforeExit.
...(libraryMode && { sendClientReports: false }),
// Disabled when telemetry is off or in library mode — the SDK registers
// beforeExit handlers for log flushing that keep the event loop alive.
enableLogs: enabled && !libraryMode,
// Disable client reports when telemetry is off or in library mode — the SDK
// registers a setInterval + beforeExit handler that keep the event loop alive.
...((libraryMode || !enabled) && { sendClientReports: false }),
// Sample all events for CLI telemetry (low volume)
tracesSampleRate: 1,
sampleRate: 1,
Expand Down Expand Up @@ -405,6 +414,12 @@ export function initSentry(
},
});

// Always remove our own previous handler on re-init.
if (currentBeforeExitHandler) {
process.removeListener("beforeExit", currentBeforeExitHandler);
currentBeforeExitHandler = null;
}

if (client?.getOptions().enabled) {
const isBun = typeof process.versions.bun !== "undefined";
const runtime = isBun ? "bun" : "node";
Expand Down Expand Up @@ -441,14 +456,7 @@ export function initSentry(
//
// Skipped in library mode — the host owns the process lifecycle.
// The library entry point calls client.flush() manually after completion.
//
// Replace previous handler on re-init (e.g., auto-login retry calls
// withTelemetry → initSentry twice) to avoid duplicate handlers with
// independent re-entry guards and stale client references.
if (!libraryMode) {
if (currentBeforeExitHandler) {
process.removeListener("beforeExit", currentBeforeExitHandler);
}
currentBeforeExitHandler = createBeforeExitHandler(client);
process.on("beforeExit", currentBeforeExitHandler);
}
Expand Down
6 changes: 6 additions & 0 deletions test/commands/auth/logout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ describe("logoutCommand.func", () => {
test("env token (SENTRY_TOKEN): shows correct env var name in error", async () => {
isAuthenticatedSpy.mockReturnValue(true);
isEnvTokenActiveSpy.mockReturnValue(true);
// Clear SENTRY_AUTH_TOKEN so SENTRY_TOKEN takes priority
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
delete process.env.SENTRY_AUTH_TOKEN;
// Set env var directly — getActiveEnvVarName() reads env vars via getEnvToken()
process.env.SENTRY_TOKEN = "sntrys_token_456";
const { context } = createContext();
Expand All @@ -138,6 +141,9 @@ describe("logoutCommand.func", () => {
expect(msg).toContain("SENTRY_TOKEN");
} finally {
delete process.env.SENTRY_TOKEN;
if (savedAuthToken !== undefined) {
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
}
}
expect(clearAuthSpy).not.toHaveBeenCalled();
});
Expand Down
6 changes: 6 additions & 0 deletions test/commands/auth/refresh.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ describe("refreshCommand.func", () => {

test("env token (SENTRY_TOKEN): throws AuthError with SENTRY_TOKEN in message", async () => {
isEnvTokenActiveSpy.mockReturnValue(true);
// Clear SENTRY_AUTH_TOKEN so SENTRY_TOKEN takes priority
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
delete process.env.SENTRY_AUTH_TOKEN;
// Set env var directly — getActiveEnvVarName() reads env vars via getEnvToken()
process.env.SENTRY_TOKEN = "sntrys_token_456";

Expand All @@ -99,6 +102,9 @@ describe("refreshCommand.func", () => {
expect((err as AuthError).message).not.toContain("SENTRY_AUTH_TOKEN");
} finally {
delete process.env.SENTRY_TOKEN;
if (savedAuthToken !== undefined) {
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
}
}

expect(refreshTokenSpy).not.toHaveBeenCalled();
Expand Down
18 changes: 18 additions & 0 deletions test/commands/auth/whoami.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,24 @@ describe("whoamiCommand.func", () => {
});

describe("unauthenticated", () => {
let getAuthConfigSpy: ReturnType<typeof spyOn>;
let savedAuthToken: string | undefined;

beforeEach(() => {
savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
delete process.env.SENTRY_AUTH_TOKEN;
getAuthConfigSpy = spyOn(dbAuth, "getAuthConfig").mockReturnValue(
undefined
);
});

afterEach(() => {
getAuthConfigSpy.mockRestore();
if (savedAuthToken !== undefined) {
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
}
});

test("throws AuthError(not_authenticated) when no token stored", async () => {
isAuthenticatedSpy.mockReturnValue(false);

Expand Down
29 changes: 28 additions & 1 deletion test/commands/log/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import {
import { listCommand } from "../../../src/commands/log/list.js";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as apiClient from "../../../src/lib/api-client.js";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as dbAuth from "../../../src/lib/db/auth.js";
import {
AuthError,
ContextError,
Expand Down Expand Up @@ -237,6 +239,23 @@ const newerLogs: SentryLog[] = [
},
];

// ============================================================================
// Auth setup — mock getAuthConfig for all tests (auth guard added in #611)
// ============================================================================

let getAuthConfigSpy: ReturnType<typeof spyOn>;

beforeEach(() => {
getAuthConfigSpy = spyOn(dbAuth, "getAuthConfig").mockReturnValue({
token: "sntrys_test",
source: "oauth" as const,
});
});

afterEach(() => {
getAuthConfigSpy.mockRestore();
});

// ============================================================================
// Standard mode (project-scoped, no trace-id positional)
// ============================================================================
Expand Down Expand Up @@ -837,7 +856,14 @@ describe("listCommand.func — flag validation", () => {

test("allows --sort newest with --follow", async () => {
// Should not throw ValidationError — the error (if any) comes from
// downstream resolution, not flag validation.
// downstream resolution, not flag validation. Mock resolution to reject
// with a non-ValidationError so we can verify flag validation passed.
const resolveOrgProjectSpy = spyOn(
resolveTarget,
"resolveOrgProjectFromArg"
).mockRejectedValueOnce(
new ContextError("Organization", "sentry log list")
);
const { context } = createMockContext();
const func = await listCommand.loader();
await expect(
Expand All @@ -847,6 +873,7 @@ describe("listCommand.func — flag validation", () => {
"my-org/my-project"
)
).rejects.not.toThrow(ValidationError);
resolveOrgProjectSpy.mockRestore();

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.

Spy cleanup skipped on assertion failure

Low Severity

The test creates resolveOrgProjectSpy and restores it only at the end of the happy path. If an assertion throws before mockRestore(), the mock remains active and can affect later tests in test/commands/log/list.test.ts, causing cascading failures and misleading CI results.

Fix in Cursor Fix in Web

});
});

Expand Down
34 changes: 26 additions & 8 deletions test/commands/project/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,16 @@ describe("fetchOrgProjectsSafe", () => {
// Clear auth token so the API client throws AuthError before making any request
await clearAuth();

await expect(fetchOrgProjectsSafe("myorg")).rejects.toThrow(AuthError);
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
delete process.env.SENTRY_AUTH_TOKEN;

try {
await expect(fetchOrgProjectsSafe("myorg")).rejects.toThrow(AuthError);
} finally {
if (savedAuthToken !== undefined) {
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
}
}
});
});

Expand Down Expand Up @@ -1234,13 +1243,22 @@ describe("handleAutoDetect", () => {
// Clear auth so getAuthToken() throws AuthError before any fetch
await clearAuth();

await expect(
handleAutoDetect("/tmp/test-project", {
limit: 30,
json: true,
fresh: false,
})
).rejects.toThrow(AuthError);
const savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
delete process.env.SENTRY_AUTH_TOKEN;

try {
await expect(
handleAutoDetect("/tmp/test-project", {
limit: 30,
json: true,
fresh: false,
})
).rejects.toThrow(AuthError);
} finally {
if (savedAuthToken !== undefined) {
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
}
}
});

test("slow path: uses full fetch when platform filter is active", async () => {
Expand Down
2 changes: 2 additions & 0 deletions test/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export function createE2EContext(
run: (args: string[]) =>
runCli(args, {
env: {
SENTRY_AUTH_TOKEN: "",
SENTRY_TOKEN: "",
[CONFIG_DIR_ENV_VAR]: configDir,
SENTRY_URL: serverUrl,
SENTRY_CLI_NO_TELEMETRY: "1",
Expand Down
11 changes: 11 additions & 0 deletions test/lib/api-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ describe("401 retry behavior", () => {
// Note: These tests use rawApiRequest which goes to control silo (sentry.io)
// and supports 401 retry with token refresh.

let savedAuthToken: string | undefined;
beforeEach(() => {
savedAuthToken = process.env.SENTRY_AUTH_TOKEN;
delete process.env.SENTRY_AUTH_TOKEN;
});
afterEach(() => {
if (savedAuthToken !== undefined) {
process.env.SENTRY_AUTH_TOKEN = savedAuthToken;
}
});

test("retries request with new token on 401 response", async () => {
const requests: RequestLog[] = [];

Expand Down
Loading
Loading