Skip to content

Commit 8008ec1

Browse files
committed
refactor: address review feedback and improve test coverage
- Use Promise.allSettled() in findEventAcrossOrgs for cleaner code - Wrap resolveEventInOrg in OrgAll case with try/catch for AuthError - Extract resolveEventTarget, resolveOrgAllTarget, resolveAutoDetectTarget helpers to reduce cognitive complexity (20 -> under 15) - Add 15 tests for the extracted resolution helpers
1 parent 3a92ddb commit 8008ec1

File tree

3 files changed

+387
-90
lines changed

3 files changed

+387
-90
lines changed

src/commands/event/view.ts

Lines changed: 119 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,119 @@ export type ResolvedEventTarget = {
151151
prefetchedEvent?: ResolvedEvent["event"];
152152
};
153153

154+
/** Options for resolving the event target */
155+
type ResolveTargetOptions = {
156+
parsed: ReturnType<typeof parseOrgProjectArg>;
157+
eventId: string;
158+
cwd: string;
159+
stderr: { write(s: string): void };
160+
};
161+
162+
/**
163+
* Resolve org/project context for the event view command.
164+
*
165+
* Handles all target types (explicit, search, org-all, auto-detect)
166+
* including cross-project fallback via the eventids endpoint.
167+
*/
168+
/** @internal Exported for testing */
169+
export async function resolveEventTarget(
170+
options: ResolveTargetOptions
171+
): Promise<ResolvedEventTarget | null> {
172+
const { parsed, eventId, cwd, stderr } = options;
173+
174+
switch (parsed.type) {
175+
case ProjectSpecificationType.Explicit:
176+
return {
177+
org: parsed.org,
178+
project: parsed.project,
179+
orgDisplay: parsed.org,
180+
projectDisplay: parsed.project,
181+
};
182+
183+
case ProjectSpecificationType.ProjectSearch: {
184+
const resolved = await resolveProjectBySlug(
185+
parsed.projectSlug,
186+
USAGE_HINT,
187+
`sentry event view <org>/${parsed.projectSlug} ${eventId}`,
188+
stderr
189+
);
190+
return {
191+
...resolved,
192+
orgDisplay: resolved.org,
193+
projectDisplay: resolved.project,
194+
};
195+
}
196+
197+
case ProjectSpecificationType.OrgAll:
198+
return resolveOrgAllTarget(parsed.org, eventId, cwd);
199+
200+
case ProjectSpecificationType.AutoDetect:
201+
return resolveAutoDetectTarget(eventId, cwd, stderr);
202+
203+
default:
204+
return null;
205+
}
206+
}
207+
208+
/**
209+
* Resolve target when only an org is known (e.g., from a Sentry event URL).
210+
* Uses the eventids endpoint to find the project, falls back to auto-detect.
211+
*/
212+
/** @internal Exported for testing */
213+
export async function resolveOrgAllTarget(
214+
org: string,
215+
eventId: string,
216+
cwd: string
217+
): Promise<ResolvedEventTarget | null> {
218+
try {
219+
const resolved = await resolveEventInOrg(org, eventId);
220+
if (resolved) {
221+
return {
222+
org: resolved.org,
223+
project: resolved.project,
224+
orgDisplay: org,
225+
projectDisplay: resolved.project,
226+
prefetchedEvent: resolved.event,
227+
};
228+
}
229+
} catch {
230+
// Auth or network errors — fall through to auto-detect
231+
}
232+
return resolveOrgAndProject({ cwd, usageHint: USAGE_HINT });
233+
}
234+
235+
/**
236+
* Resolve target via auto-detect cascade, falling back to cross-project
237+
* event search across all accessible orgs.
238+
*/
239+
/** @internal Exported for testing */
240+
export async function resolveAutoDetectTarget(
241+
eventId: string,
242+
cwd: string,
243+
stderr: { write(s: string): void }
244+
): Promise<ResolvedEventTarget | null> {
245+
const autoTarget = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT });
246+
if (autoTarget) {
247+
return autoTarget;
248+
}
249+
250+
const resolved = await findEventAcrossOrgs(eventId);
251+
if (resolved) {
252+
stderr.write(
253+
`Tip: Found event in ${resolved.org}/${resolved.project}. ` +
254+
`Run "sentry project set ${resolved.org}/${resolved.project}" to set as default.\n`
255+
);
256+
return {
257+
org: resolved.org,
258+
project: resolved.project,
259+
orgDisplay: resolved.org,
260+
projectDisplay: resolved.project,
261+
prefetchedEvent: resolved.event,
262+
};
263+
}
264+
return null;
265+
}
266+
154267
export const viewCommand = buildCommand({
155268
docs: {
156269
brief: "View details of a specific event",
@@ -197,85 +310,12 @@ export const viewCommand = buildCommand({
197310
const { eventId, targetArg } = parsePositionalArgs(args);
198311
const parsed = parseOrgProjectArg(targetArg);
199312

200-
let target: ResolvedEventTarget | null = null;
201-
202-
switch (parsed.type) {
203-
case ProjectSpecificationType.Explicit:
204-
target = {
205-
org: parsed.org,
206-
project: parsed.project,
207-
orgDisplay: parsed.org,
208-
projectDisplay: parsed.project,
209-
};
210-
break;
211-
212-
case ProjectSpecificationType.ProjectSearch: {
213-
const resolved = await resolveProjectBySlug(
214-
parsed.projectSlug,
215-
USAGE_HINT,
216-
`sentry event view <org>/${parsed.projectSlug} ${eventId}`,
217-
this.stderr
218-
);
219-
target = {
220-
...resolved,
221-
orgDisplay: resolved.org,
222-
projectDisplay: resolved.project,
223-
};
224-
break;
225-
}
226-
227-
case ProjectSpecificationType.OrgAll: {
228-
// Org-only (e.g., from a Sentry event URL that has no project slug).
229-
// Use the org from the URL + event ID to resolve the project directly.
230-
const resolved = await resolveEventInOrg(parsed.org, eventId);
231-
if (resolved) {
232-
target = {
233-
org: resolved.org,
234-
project: resolved.project,
235-
orgDisplay: parsed.org,
236-
projectDisplay: resolved.project,
237-
prefetchedEvent: resolved.event,
238-
};
239-
} else {
240-
// Event not found in the given org — fall back to full auto-detect
241-
target = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT });
242-
}
243-
break;
244-
}
245-
246-
case ProjectSpecificationType.AutoDetect: {
247-
// Try the standard resolution cascade first (env vars, config defaults,
248-
// DSN detection, directory name inference). If none apply, fan out
249-
// across all accessible orgs using the eventids endpoint.
250-
const autoTarget = await resolveOrgAndProject({
251-
cwd,
252-
usageHint: USAGE_HINT,
253-
});
254-
if (autoTarget) {
255-
target = autoTarget;
256-
} else {
257-
const resolved = await findEventAcrossOrgs(eventId);
258-
if (resolved) {
259-
this.stderr.write(
260-
`Tip: Found event in ${resolved.org}/${resolved.project}. ` +
261-
`Run "sentry project set ${resolved.org}/${resolved.project}" to set as default.\n`
262-
);
263-
target = {
264-
org: resolved.org,
265-
project: resolved.project,
266-
orgDisplay: resolved.org,
267-
projectDisplay: resolved.project,
268-
prefetchedEvent: resolved.event,
269-
};
270-
}
271-
}
272-
break;
273-
}
274-
275-
default:
276-
// Exhaustive check - should never reach here
277-
throw new ContextError("Organization and project", USAGE_HINT);
278-
}
313+
const target = await resolveEventTarget({
314+
parsed,
315+
eventId,
316+
cwd,
317+
stderr: this.stderr,
318+
});
279319

280320
if (!target) {
281321
throw new ContextError("Organization and project", USAGE_HINT);

src/lib/api-client.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,18 +1261,16 @@ export async function findEventAcrossOrgs(
12611261
): Promise<ResolvedEvent | null> {
12621262
const orgs = await listOrganizations();
12631263

1264-
const results = await Promise.all(
1265-
orgs.map(async (org) => {
1266-
try {
1267-
return await resolveEventInOrg(org.slug, eventId);
1268-
} catch {
1269-
// 404 or permission errors — event not in this org
1270-
return null;
1271-
}
1272-
})
1264+
const results = await Promise.allSettled(
1265+
orgs.map((org) => resolveEventInOrg(org.slug, eventId))
12731266
);
12741267

1275-
return results.find((r): r is ResolvedEvent => r !== null) ?? null;
1268+
for (const result of results) {
1269+
if (result.status === "fulfilled" && result.value !== null) {
1270+
return result.value;
1271+
}
1272+
}
1273+
return null;
12761274
}
12771275

12781276
/**

0 commit comments

Comments
 (0)