Skip to content

Commit d67b1eb

Browse files
committed
fix: address PR review comments for positional args refactor
- Fix foundProject.organization?.slug bug (use orgSlug instead) - Make formatMultipleProjectsFooter generic (not hardcoded to issue list) - Fix ContextError misuse producing garbled messages: - Use ValidationError for 'multiple orgs' error - Use ValidationError for 'invalid target' error - Use noun phrase 'Specific project' for OrgAll case - Update dsn/errors.ts to use generic 'sentry <command>' in hints - Update test expectations to match new messages
1 parent 8c4efc3 commit d67b1eb

3 files changed

Lines changed: 21 additions & 35 deletions

File tree

src/commands/event/view.ts

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
spansFlag,
1414
} from "../../lib/arg-parsing.js";
1515
import { openInBrowser } from "../../lib/browser.js";
16-
import { ContextError } from "../../lib/errors.js";
16+
import { ContextError, ValidationError } from "../../lib/errors.js";
1717
import { formatEventDetails, writeJson } from "../../lib/formatters/index.js";
1818
import { resolveOrgAndProject } from "../../lib/resolve-target.js";
1919
import { buildEventSearchUrl } from "../../lib/sentry-urls.js";
@@ -116,30 +116,19 @@ async function resolveFromProjectSearch(
116116
]);
117117
}
118118
if (found.length > 1) {
119-
const alternatives = found.map(
120-
(p) => `${p.organization?.slug ?? "unknown"}/${p.slug}`
121-
);
122-
throw new ContextError(
123-
`Project "${projectSlug}" exists in multiple organizations`,
124-
`sentry event view <org>/${projectSlug} ${eventId}`,
125-
alternatives
126-
);
127-
}
128-
const foundProject = found[0];
129-
if (!foundProject) {
130-
throw new ContextError(`Project "${projectSlug}" not found`, USAGE_HINT);
131-
}
132-
const orgSlug = foundProject.organization?.slug;
133-
if (!orgSlug) {
134-
throw new ContextError(
135-
`Could not determine organization for project "${projectSlug}"`,
136-
USAGE_HINT
119+
const orgList = found.map((p) => ` ${p.orgSlug}/${p.slug}`).join("\n");
120+
throw new ValidationError(
121+
`Project "${projectSlug}" exists in multiple organizations.\n\n` +
122+
`Specify the organization:\n${orgList}\n\n` +
123+
`Example: sentry event view <org>/${projectSlug} ${eventId}`
137124
);
138125
}
126+
// Safe assertion: length is exactly 1 after the checks above
127+
const foundProject = found[0] as (typeof found)[0];
139128
return {
140-
org: orgSlug,
129+
org: foundProject.orgSlug,
141130
project: foundProject.slug,
142-
orgDisplay: orgSlug,
131+
orgDisplay: foundProject.orgSlug,
143132
projectDisplay: foundProject.slug,
144133
};
145134
}
@@ -207,18 +196,15 @@ export const viewCommand = buildCommand({
207196
break;
208197

209198
case ProjectSpecificationType.OrgAll:
210-
throw new ContextError(
211-
"A specific project is required for event view",
212-
USAGE_HINT
213-
);
199+
throw new ContextError("Specific project", USAGE_HINT);
214200

215201
case ProjectSpecificationType.AutoDetect:
216202
target = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT });
217203
break;
218204

219205
default:
220206
// Exhaustive check - should never reach here
221-
throw new ContextError("Invalid target specification", USAGE_HINT);
207+
throw new ValidationError("Invalid target specification");
222208
}
223209

224210
if (!target) {

src/lib/dsn/errors.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export function formatConflictError(result: DsnDetectionResult): string {
3131
});
3232

3333
lines.push("To resolve, specify which project to use:");
34-
lines.push(" sentry issue list <org>/<project>");
34+
lines.push(" sentry <command> <org>/<project>");
3535
lines.push("");
3636
lines.push("Or set a default project:");
3737
lines.push(" sentry config set defaults.org <org>");
@@ -91,7 +91,7 @@ export async function formatNoDsnError(
9191
);
9292
lines.push("");
9393
lines.push("3. Specify project explicitly:");
94-
lines.push(" sentry issue list <org>/<project>");
94+
lines.push(" sentry <command> <org>/<project>");
9595
lines.push("");
9696
lines.push("4. Set default project:");
9797
lines.push(" sentry config set defaults.org <org>");
@@ -119,7 +119,7 @@ export function formatResolutionError(error: Error, dsnRaw: string): string {
119119
" - The DSN is invalid or expired",
120120
"",
121121
"Try specifying the project explicitly:",
122-
" sentry issue list <org>/<project>",
122+
" sentry <command> <org>/<project>",
123123
];
124124

125125
return lines.join("\n");
@@ -147,7 +147,7 @@ type ProjectInfo = {
147147
* Found 2 Sentry projects:
148148
* • my-org / frontend (from packages/frontend/.env)
149149
* • my-org / backend (from src/sentry.ts)
150-
* Specify a project: sentry issue list <org>/<project>
150+
* Use <org>/<project> to target a specific project.
151151
* ```
152152
*/
153153
export function formatMultipleProjectsFooter(projects: ProjectInfo[]): string {
@@ -162,7 +162,7 @@ export function formatMultipleProjectsFooter(projects: ProjectInfo[]): string {
162162
lines.push(` • ${p.orgDisplay} / ${p.projectDisplay}${source}`);
163163
}
164164

165-
lines.push("Specify a project: sentry issue list <org>/<project>");
165+
lines.push("Use <org>/<project> to target a specific project.");
166166

167167
return lines.join("\n");
168168
}

test/lib/dsn/errors.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe("formatConflictError", () => {
5959
expect(error).toContain("def456@o789.ingest.sentry.io/101112");
6060
expect(error).toContain("Project ID: 456");
6161
expect(error).toContain("Project ID: 101112");
62-
expect(error).toContain("sentry issue list <org>/<project>");
62+
expect(error).toContain("sentry <command> <org>/<project>");
6363
expect(error).toContain("sentry config set defaults.org");
6464
});
6565

@@ -183,7 +183,7 @@ describe("formatNoDsnError", () => {
183183
expect(error).toContain(".env files");
184184
expect(error).toContain("JavaScript/TypeScript source code");
185185
expect(error).toContain("export SENTRY_DSN=");
186-
expect(error).toContain("sentry issue list <org>/<project>");
186+
expect(error).toContain("sentry <command> <org>/<project>");
187187
expect(error).toContain("sentry config set defaults.org");
188188
});
189189

@@ -245,7 +245,7 @@ describe("formatResolutionError", () => {
245245
expect(formatted).toContain("You don't have access to this project");
246246
expect(formatted).toContain("self-hosted Sentry instance");
247247
expect(formatted).toContain("invalid or expired");
248-
expect(formatted).toContain("sentry issue list <org>/<project>");
248+
expect(formatted).toContain("sentry <command> <org>/<project>");
249249
});
250250

251251
test("formats error with access denied message", () => {
@@ -302,7 +302,7 @@ describe("formatMultipleProjectsFooter", () => {
302302
);
303303
expect(footer).toContain("• my-org / backend (from src/sentry.ts)");
304304
expect(footer).toContain(
305-
"Specify a project: sentry issue list <org>/<project>"
305+
"Use <org>/<project> to target a specific project."
306306
);
307307
});
308308

0 commit comments

Comments
 (0)