Skip to content

Commit 61619e6

Browse files
committed
Merge branch 'main' into feat/project-create
2 parents 6294862 + baaa3d3 commit 61619e6

20 files changed

+501
-275
lines changed

AGENTS.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,37 @@ Minimal comments, maximum clarity. Comments explain **intent and reasoning**, no
383383
| Unit tests | `*.test.ts` | `test/` (mirrors `src/`) |
384384
| E2E tests | `*.test.ts` | `test/e2e/` |
385385

386+
### Test Environment Isolation (CRITICAL)
387+
388+
Tests that need a database or config directory **must** use `useTestConfigDir()` from `test/helpers.ts`. This helper:
389+
- Creates a unique temp directory in `beforeEach`
390+
- Sets `SENTRY_CONFIG_DIR` to point at it
391+
- **Restores** (never deletes) the env var in `afterEach`
392+
- Closes the database and cleans up temp files
393+
394+
**NEVER** do any of these in test files:
395+
- `delete process.env.SENTRY_CONFIG_DIR`This pollutes other test files that load after yours
396+
- `const baseDir = process.env[CONFIG_DIR_ENV_VAR]!` at module scopeThis captures a value that may be stale
397+
- Manual `beforeEach`/`afterEach` that sets/deletes `SENTRY_CONFIG_DIR`
398+
399+
**Why**: Bun runs test files **sequentially in one thread** (loadrun all testsload next file). If your `afterEach` deletes the env var, the next file's module-level code reads `undefined`, causing `TypeError: The "paths[0]" property must be of type string`.
400+
401+
```typescript
402+
// CORRECT: Use the helper
403+
import { useTestConfigDir } from "../helpers.js";
404+
405+
const getConfigDir = useTestConfigDir("my-test-prefix-");
406+
407+
// If you need the directory path in a test:
408+
test("example", () => {
409+
const dir = getConfigDir();
410+
});
411+
412+
// WRONG: Manual env var management
413+
beforeEach(() => { process.env.SENTRY_CONFIG_DIR = tmpDir; });
414+
afterEach(() => { delete process.env.SENTRY_CONFIG_DIR; }); // BUG!
415+
```
416+
386417
### Property-Based Testing
387418

388419
Use property-based tests when verifying invariants that should hold for **any valid input**.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
"engines": {
6060
"node": ">=22"
6161
},
62-
"packageManager": "bun@1.3.3",
62+
"packageManager": "bun@1.3.9",
6363
"patchedDependencies": {
6464
"@stricli/core@1.2.5": "patches/@stricli%2Fcore@1.2.5.patch",
6565
"@sentry/core@10.38.0": "patches/@sentry%2Fcore@10.38.0.patch"

src/commands/cli/setup.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ function printWelcomeMessage(
171171
log(`Installed sentry v${version} to ${binaryPath}`);
172172
log("");
173173
log("Get started:");
174-
log(" sentry login Authenticate with Sentry");
174+
log(" sentry auth login Authenticate with Sentry");
175175
log(" sentry --help See all available commands");
176176
log("");
177177
log("https://cli.sentry.dev");

src/lib/oauth.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,20 @@ const SENTRY_URL = process.env.SENTRY_URL ?? "https://sentry.io";
2424
* Build-time: Injected via Bun.build({ define: { SENTRY_CLIENT_ID: "..." } })
2525
* Runtime: Can be overridden via SENTRY_CLIENT_ID env var (for self-hosted)
2626
*
27+
* Read at call time (not module load time) so tests can set process.env.SENTRY_CLIENT_ID
28+
* after module initialization.
29+
*
2730
* @see script/build.ts
2831
*/
2932
declare const SENTRY_CLIENT_ID_BUILD: string | undefined;
30-
const SENTRY_CLIENT_ID =
31-
process.env.SENTRY_CLIENT_ID ??
32-
(typeof SENTRY_CLIENT_ID_BUILD !== "undefined" ? SENTRY_CLIENT_ID_BUILD : "");
33+
function getClientId(): string {
34+
return (
35+
process.env.SENTRY_CLIENT_ID ??
36+
(typeof SENTRY_CLIENT_ID_BUILD !== "undefined"
37+
? SENTRY_CLIENT_ID_BUILD
38+
: "")
39+
);
40+
}
3341

3442
// OAuth scopes requested for the CLI
3543
const SCOPES = [
@@ -85,7 +93,8 @@ async function fetchWithConnectionError(
8593

8694
/** Request a device code from Sentry's device authorization endpoint */
8795
function requestDeviceCode() {
88-
if (!SENTRY_CLIENT_ID) {
96+
const clientId = getClientId();
97+
if (!clientId) {
8998
throw new ConfigError(
9099
"SENTRY_CLIENT_ID is required for authentication",
91100
"Set SENTRY_CLIENT_ID environment variable or use a pre-built binary"
@@ -99,7 +108,7 @@ function requestDeviceCode() {
99108
method: "POST",
100109
headers: { "Content-Type": "application/x-www-form-urlencoded" },
101110
body: new URLSearchParams({
102-
client_id: SENTRY_CLIENT_ID,
111+
client_id: clientId,
103112
scope: SCOPES,
104113
}),
105114
}
@@ -142,7 +151,7 @@ function pollForToken(deviceCode: string): Promise<TokenResponse> {
142151
method: "POST",
143152
headers: { "Content-Type": "application/x-www-form-urlencoded" },
144153
body: new URLSearchParams({
145-
client_id: SENTRY_CLIENT_ID,
154+
client_id: getClientId(),
146155
device_code: deviceCode,
147156
grant_type: "urn:ietf:params:oauth:grant-type:device_code",
148157
}),
@@ -313,7 +322,8 @@ export async function setApiToken(token: string): Promise<void> {
313322
export function refreshAccessToken(
314323
refreshToken: string
315324
): Promise<TokenResponse> {
316-
if (!SENTRY_CLIENT_ID) {
325+
const clientId = getClientId();
326+
if (!clientId) {
317327
throw new ConfigError(
318328
"SENTRY_CLIENT_ID is required for token refresh",
319329
"Set SENTRY_CLIENT_ID environment variable or use a pre-built binary"
@@ -327,7 +337,7 @@ export function refreshAccessToken(
327337
method: "POST",
328338
headers: { "Content-Type": "application/x-www-form-urlencoded" },
329339
body: new URLSearchParams({
330-
client_id: SENTRY_CLIENT_ID,
340+
client_id: clientId,
331341
grant_type: "refresh_token",
332342
refresh_token: refreshToken,
333343
}),

src/lib/telemetry.ts

Lines changed: 86 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,36 @@ async function initTelemetryContext(): Promise<void> {
4949
}
5050
}
5151

52+
/**
53+
* Mark the active session as crashed.
54+
*
55+
* Checks both current scope and isolation scope since processSessionIntegration
56+
* stores the session on the isolation scope. Called when a command error
57+
* propagates through withTelemetry — the SDK auto-marks crashes for truly
58+
* uncaught exceptions (mechanism.handled === false), but command errors need
59+
* explicit marking.
60+
*
61+
* @internal Exported for testing
62+
*/
63+
export function markSessionCrashed(): void {
64+
const session =
65+
Sentry.getCurrentScope().getSession() ??
66+
Sentry.getIsolationScope().getSession();
67+
if (session) {
68+
session.status = "crashed";
69+
}
70+
}
71+
5272
/**
5373
* Wrap CLI execution with telemetry tracking.
5474
*
55-
* Creates a Sentry session and span for the command execution.
56-
* Captures any unhandled exceptions and reports them.
75+
* Creates a Sentry span for the command execution and captures exceptions.
76+
* Session lifecycle is managed by the SDK's processSessionIntegration
77+
* (started during Sentry.init) and a beforeExit handler (registered in
78+
* initSentry) that ends healthy sessions and flushes events. This ensures
79+
* sessions are reliably tracked even for unhandled rejections and other
80+
* paths that bypass this function's try/catch.
81+
*
5782
* Telemetry can be disabled via SENTRY_CLI_NO_TELEMETRY=1 env var.
5883
*
5984
* @param callback - The CLI execution function to wrap, receives the span for naming
@@ -71,9 +96,6 @@ export async function withTelemetry<T>(
7196
// Initialize user and instance context
7297
await initTelemetryContext();
7398

74-
Sentry.startSession();
75-
Sentry.captureSession();
76-
7799
try {
78100
return await Sentry.startSpanManual(
79101
{ name: "cli.command", op: "cli.command", forceTransaction: true },
@@ -92,31 +114,47 @@ export async function withTelemetry<T>(
92114
e instanceof AuthError && e.reason === "not_authenticated";
93115
if (!isExpectedAuthState) {
94116
Sentry.captureException(e);
95-
const session = Sentry.getCurrentScope().getSession();
96-
if (session) {
97-
session.status = "crashed";
98-
}
117+
markSessionCrashed();
99118
}
100119
throw e;
101-
} finally {
102-
Sentry.endSession();
103-
// Flush with a timeout to ensure events are sent before process exits
104-
try {
105-
await client.flush(3000);
106-
} catch {
107-
// Ignore flush errors - telemetry should never block CLI
108-
}
109120
}
110121
}
111122

112123
/**
113-
* Initialize Sentry for telemetry.
124+
* Create a beforeExit handler that ends healthy sessions and flushes events.
114125
*
115-
* @param enabled - Whether telemetry is enabled
116-
* @returns The Sentry client, or undefined if initialization failed
126+
* The SDK's processSessionIntegration only ends non-OK sessions (crashed/errored).
127+
* This handler complements it by ending OK sessions (clean exit → 'exited')
128+
* and flushing pending events. Includes a re-entry guard since flush is async
129+
* and causes beforeExit to re-fire when complete.
130+
*
131+
* @param client - The Sentry client to flush
132+
* @returns A handler function for process.on("beforeExit")
117133
*
118134
* @internal Exported for testing
119135
*/
136+
export function createBeforeExitHandler(client: Sentry.BunClient): () => void {
137+
let isFlushing = false;
138+
return () => {
139+
if (isFlushing) {
140+
return;
141+
}
142+
isFlushing = true;
143+
144+
const session = Sentry.getIsolationScope().getSession();
145+
if (session?.status === "ok") {
146+
Sentry.endSession();
147+
}
148+
149+
// Flush pending events before exit. Convert PromiseLike to Promise
150+
// for proper error handling. The async work causes beforeExit to
151+
// re-fire when complete, which the isFlushing guard handles.
152+
Promise.resolve(client.flush(3000)).catch(() => {
153+
// Ignore flush errors — telemetry should never block CLI exit
154+
});
155+
};
156+
}
157+
120158
/**
121159
* Integrations to exclude for CLI.
122160
* These add overhead without benefit for short-lived CLI processes.
@@ -128,6 +166,17 @@ const EXCLUDED_INTEGRATIONS = new Set([
128166
"Modules", // Lists all loaded modules - unnecessary for CLI telemetry
129167
]);
130168

169+
/** Current beforeExit handler, tracked so it can be replaced on re-init */
170+
let currentBeforeExitHandler: (() => void) | null = null;
171+
172+
/**
173+
* Initialize Sentry for telemetry.
174+
*
175+
* @param enabled - Whether telemetry is enabled
176+
* @returns The Sentry client, or undefined if initialization failed
177+
*
178+
* @internal Exported for testing
179+
*/
131180
export function initSentry(enabled: boolean): Sentry.BunClient | undefined {
132181
const environment = process.env.NODE_ENV ?? "development";
133182

@@ -187,6 +236,23 @@ export function initSentry(enabled: boolean): Sentry.BunClient | undefined {
187236

188237
// Tag whether targeting self-hosted Sentry (not SaaS)
189238
Sentry.setTag("is_self_hosted", !isSentrySaasUrl(getSentryBaseUrl()));
239+
240+
// End healthy sessions and flush events when the event loop drains.
241+
// The SDK's processSessionIntegration starts a session during init and
242+
// registers its own beforeExit handler that ends non-OK (crashed/errored)
243+
// sessions. We complement it by ending OK sessions (clean exit → 'exited')
244+
// and flushing pending events. This covers unhandled rejections and other
245+
// paths that bypass withTelemetry's try/catch.
246+
// Ref: https://nodejs.org/api/process.html#event-beforeexit
247+
//
248+
// Replace previous handler on re-init (e.g., auto-login retry calls
249+
// withTelemetry → initSentry twice) to avoid duplicate handlers with
250+
// independent re-entry guards and stale client references.
251+
if (currentBeforeExitHandler) {
252+
process.removeListener("beforeExit", currentBeforeExitHandler);
253+
}
254+
currentBeforeExitHandler = createBeforeExitHandler(client);
255+
process.on("beforeExit", currentBeforeExitHandler);
190256
}
191257

192258
return client;

0 commit comments

Comments
 (0)