Skip to content

Commit 71ddebe

Browse files
committed
fix(args): address BugBot review issues in URL parsing
- Event URLs no longer trigger OrgAll → ContextError; instead they auto-detect org/project after SENTRY_URL is set (HIGH) - SaaS URLs now clear stale SENTRY_URL so API calls use default routing instead of a leftover self-hosted value (MEDIUM) - Non-issue URLs (traces, project settings) in parseIssueArg now throw ValidationError instead of falling through to garbage slash-based parsing (LOW)
1 parent 2657c09 commit 71ddebe

File tree

6 files changed

+63
-8
lines changed

6 files changed

+63
-8
lines changed

src/commands/event/view.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,10 @@ export function parsePositionalArgs(args: string[]): {
9999
if (urlParsed) {
100100
applySentryUrlContext(urlParsed.baseUrl);
101101
if (urlParsed.eventId) {
102-
// Event URL: use org as target, eventId from the URL
103-
return { eventId: urlParsed.eventId, targetArg: `${urlParsed.org}/` };
102+
// Event URL: eventId from the URL, auto-detect org/project.
103+
// SENTRY_URL is already set for self-hosted; org/project will be
104+
// resolved via DSN detection or cached defaults.
105+
return { eventId: urlParsed.eventId, targetArg: undefined };
104106
}
105107
// URL recognized but no eventId — not valid for event view
106108
throw new ContextError(

src/lib/arg-parsing.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* project list) and single-item commands (issue view, explain, plan).
77
*/
88

9+
import { ValidationError } from "./errors.js";
910
import type { ParsedSentryUrl } from "./sentry-url-parser.js";
1011
import { applySentryUrlContext, parseSentryUrl } from "./sentry-url-parser.js";
1112
import { isAllDigits } from "./utils.js";
@@ -353,8 +354,11 @@ export function parseIssueArg(arg: string): ParsedIssueArg {
353354
if (result) {
354355
return result;
355356
}
356-
// URL recognized but no issue ID — fall through to normal parsing
357-
// (shouldn't happen for issue commands, but defensive)
357+
// URL recognized but no issue ID (e.g., trace or project settings URL)
358+
throw new ValidationError(
359+
"This Sentry URL does not contain an issue ID. Use an issue URL like:\n" +
360+
" https://sentry.io/organizations/{org}/issues/{id}/"
361+
);
358362
}
359363

360364
// 1. Pure numeric → direct fetch by ID

src/lib/sentry-url-parser.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ export function parseSentryUrl(input: string): ParsedSentryUrl | null {
131131
*/
132132
export function applySentryUrlContext(baseUrl: string): void {
133133
if (isSentrySaasUrl(baseUrl)) {
134+
// Clear any self-hosted URL so API calls fall back to default SaaS routing.
135+
// Without this, a stale SENTRY_URL would route SaaS requests to the wrong host.
136+
process.env.SENTRY_URL = undefined;
134137
return;
135138
}
136139
process.env.SENTRY_URL = baseUrl;

test/commands/event/view.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,20 +107,21 @@ describe("parsePositionalArgs", () => {
107107
}
108108
});
109109

110-
test("event URL extracts eventId and org as target", () => {
110+
test("event URL extracts eventId, auto-detects org/project", () => {
111111
const result = parsePositionalArgs([
112112
"https://sentry.io/organizations/my-org/issues/32886/events/abc123def456/",
113113
]);
114114
expect(result.eventId).toBe("abc123def456");
115-
expect(result.targetArg).toBe("my-org/");
115+
expect(result.targetArg).toBeUndefined();
116116
});
117117

118-
test("self-hosted event URL extracts eventId and org", () => {
118+
test("self-hosted event URL extracts eventId, sets SENTRY_URL", () => {
119119
const result = parsePositionalArgs([
120120
"https://sentry.example.com/organizations/acme/issues/999/events/deadbeef/",
121121
]);
122122
expect(result.eventId).toBe("deadbeef");
123-
expect(result.targetArg).toBe("acme/");
123+
expect(result.targetArg).toBeUndefined();
124+
expect(process.env.SENTRY_URL).toBe("https://sentry.example.com");
124125
});
125126

126127
test("issue URL without event ID throws ContextError", () => {

test/lib/arg-parsing.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
parseIssueArg,
1212
parseOrgProjectArg,
1313
} from "../../src/lib/arg-parsing.js";
14+
import { ValidationError } from "../../src/lib/errors.js";
1415

1516
describe("parseOrgProjectArg", () => {
1617
// Representative examples for documentation (invariants covered by property tests)
@@ -229,6 +230,38 @@ describe("parseIssueArg", () => {
229230
numericId: "32886",
230231
});
231232
});
233+
234+
test("trace URL throws ValidationError (no issue ID in URL)", () => {
235+
expect(() =>
236+
parseIssueArg(
237+
"https://sentry.io/organizations/my-org/traces/a4d1aae7216b47ff/"
238+
)
239+
).toThrow(ValidationError);
240+
});
241+
242+
test("org-only URL throws ValidationError (no issue ID in URL)", () => {
243+
expect(() =>
244+
parseIssueArg("https://sentry.io/organizations/my-org/")
245+
).toThrow(ValidationError);
246+
});
247+
248+
test("project settings URL throws ValidationError (no issue ID in URL)", () => {
249+
expect(() =>
250+
parseIssueArg("https://sentry.io/settings/my-org/projects/backend/")
251+
).toThrow(ValidationError);
252+
});
253+
254+
test("non-issue URL error mentions issue URL format", () => {
255+
try {
256+
parseIssueArg("https://sentry.io/organizations/my-org/traces/abc/");
257+
expect.unreachable("Should have thrown");
258+
} catch (error) {
259+
expect(error).toBeInstanceOf(ValidationError);
260+
expect((error as ValidationError).message).toContain(
261+
"does not contain an issue ID"
262+
);
263+
}
264+
});
232265
});
233266

234267
// Edge cases - document tricky behaviors

test/lib/sentry-url-parser.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,4 +271,16 @@ describe("applySentryUrlContext", () => {
271271
applySentryUrlContext("https://sentry.acme.internal:9000");
272272
expect(process.env.SENTRY_URL).toBe("https://sentry.acme.internal:9000");
273273
});
274+
275+
test("clears existing SENTRY_URL when SaaS URL is detected", () => {
276+
process.env.SENTRY_URL = "https://sentry.example.com";
277+
applySentryUrlContext("https://sentry.io");
278+
expect(process.env.SENTRY_URL).toBeUndefined();
279+
});
280+
281+
test("clears existing SENTRY_URL when SaaS subdomain is detected", () => {
282+
process.env.SENTRY_URL = "https://sentry.example.com";
283+
applySentryUrlContext("https://us.sentry.io");
284+
expect(process.env.SENTRY_URL).toBeUndefined();
285+
});
274286
});

0 commit comments

Comments
 (0)