Skip to content

Commit 4ca3801

Browse files
committed
refactor(log): extract shared displayTraceLogs helper (#329)
Extract duplicated trace-log display logic into a shared displayTraceLogs() function in src/lib/formatters/log.ts. Both 'sentry log list --trace' and 'sentry trace logs' now use the same helper for JSON output, empty state, and table formatting. This also normalizes JSON output for trace/logs.ts to use chronological order (oldest-first), consistent with all other JSON outputs in the CLI.
1 parent fffc227 commit 4ca3801

File tree

5 files changed

+76
-48
lines changed

5 files changed

+76
-48
lines changed

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ mock.module("./some-module", () => ({
677677
* **PR review workflow: reply, resolve, amend, force-push**: PR review workflow: (1) Read unresolved threads via GraphQL, (2) make code changes, (3) run lint+typecheck+tests, (4) create a SEPARATE commit per review round (not amend) for incremental review, (5) push normally, (6) reply to comments via REST API, (7) resolve threads via GraphQL \`resolveReviewThread\`. Only amend+force-push when user explicitly asks or pre-commit hook modified files.
678678
679679
<!-- lore:019cb858-e780-7945-a0ee-a01f85be5585 -->
680-
* **Testing infinite loops (while-true) in streaming functions via AuthError escape hatch**: Follow-mode streaming in \`sentry log list\` uses \`setTimeout\`-based recursive scheduling. \`scheduleNextPoll()\` sets a timer; SIGINT handler calls \`clearTimeout()\` + \`resolve()\`. No \`process.exit(0)\` needed — clearing the timer drains the event loop naturally. A \`stopped\` boolean guard prevents orphaned poll loops when SIGINT fires during an in-flight fetch. Both \`scheduleNextPoll()\` and \`poll()\` check \`stopped\` before doing work. \`Bun.sleep()\` cannot be cancelled (no AbortSignal support), so \`setTimeout\`/\`clearTimeout\` is required. The Promise constructor uses an async IIFE for the initial fetch (async/await inside \`new Promise\` with external \`resolve\`/\`reject\` for SIGINT). Tests use \`interceptSigint()\` helper that captures \`process.once('SIGINT', handler)\` via spy, then \`trigger()\` invokes it directly. \`executeFollowMode\<T>(config: FollowConfig\<T>)\` is generic — parameterized by a single \`fetch(statsPeriod, afterTimestamp?)\` callback plus \`extractNew\`.
680+
* **Testing infinite loops (while-true) in streaming functions via AuthError escape hatch**: Follow-mode streaming in \`sentry log list\` uses \`setTimeout\`-based recursive scheduling. \`scheduleNextPoll()\` sets a timer; SIGINT handler calls \`clearTimeout()\` + \`resolve()\`. No \`process.exit(0)\` needed — clearing the timer drains the event loop naturally. A \`stopped\` boolean guard prevents orphaned poll loops when SIGINT fires during an in-flight fetch. Both \`scheduleNextPoll()\` and \`poll()\` check \`stopped\` before doing work. \`Bun.sleep()\` cannot be cancelled (no AbortSignal support), so \`setTimeout\`/\`clearTimeout\` is required. The Promise constructor uses fire-and-forget \`.then()/.catch()\` for the initial fetch — cannot use async IIFE because \`resolve\` must remain callable by the SIGINT handler (\`stop\`) at any time; awaiting would block the executor. Tests use \`interceptSigint()\` helper that captures \`process.once('SIGINT', handler)\` via spy, then \`trigger()\` invokes it directly. \`executeFollowMode\<T>(config: FollowConfig\<T>)\` is generic — parameterized by a single \`fetch(statsPeriod, afterTimestamp?)\` callback plus \`extractNew\`.
681681
682682
### Preference
683683

src/commands/log/list.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { AuthError, ContextError, stringifyUnknown } from "../../lib/errors.js";
1515
import {
1616
buildLogRowCells,
1717
createLogStreamingTable,
18+
displayTraceLogs,
1819
formatLogRow,
1920
formatLogsHeader,
2021
formatLogTable,
@@ -341,26 +342,16 @@ async function executeTraceSingleFetch(
341342
statsPeriod: DEFAULT_TRACE_PERIOD,
342343
});
343344

344-
if (flags.json) {
345-
writeJson(stdout, [...logs].reverse());
346-
return;
347-
}
348-
349-
if (logs.length === 0) {
350-
stdout.write(
345+
displayTraceLogs({
346+
stdout,
347+
logs,
348+
traceId,
349+
limit: flags.limit,
350+
asJson: flags.json,
351+
emptyMessage:
351352
`No logs found for trace ${traceId} in the last ${DEFAULT_TRACE_PERIOD}.\n\n` +
352-
"Try 'sentry trace logs' for more options (e.g., --period 30d).\n"
353-
);
354-
return;
355-
}
356-
357-
const chronological = [...logs].reverse();
358-
stdout.write(formatLogTable(chronological, false));
359-
360-
const hasMore = logs.length >= flags.limit;
361-
const countText = `Showing ${logs.length} log${logs.length === 1 ? "" : "s"} for trace ${traceId}.`;
362-
const tip = hasMore ? " Use --limit to show more." : "";
363-
writeFooter(stdout, `${countText}${tip}`);
353+
"Try 'sentry trace logs' for more options (e.g., --period 30d).\n",
354+
});
364355
}
365356

366357
export const listCommand = buildListCommand("log", {

src/commands/trace/logs.ts

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,7 @@ import { validateLimit } from "../../lib/arg-parsing.js";
1010
import { openInBrowser } from "../../lib/browser.js";
1111
import { buildCommand } from "../../lib/command.js";
1212
import { ContextError } from "../../lib/errors.js";
13-
import {
14-
formatLogTable,
15-
writeFooter,
16-
writeJson,
17-
} from "../../lib/formatters/index.js";
13+
import { displayTraceLogs } from "../../lib/formatters/index.js";
1814
import { resolveOrg } from "../../lib/resolve-target.js";
1915
import { buildTraceUrl } from "../../lib/sentry-urls.js";
2016
import { validateTraceId } from "../../lib/trace-id.js";
@@ -205,27 +201,15 @@ export const logsCommand = buildCommand({
205201
query: flags.query,
206202
});
207203

208-
if (flags.json) {
209-
writeJson(stdout, logs);
210-
return;
211-
}
212-
213-
if (logs.length === 0) {
214-
stdout.write(
204+
displayTraceLogs({
205+
stdout,
206+
logs,
207+
traceId,
208+
limit: flags.limit,
209+
asJson: flags.json,
210+
emptyMessage:
215211
`No logs found for trace ${traceId} in the last ${flags.period}.\n\n` +
216-
`Try a longer period: sentry trace logs --period 30d ${traceId}\n`
217-
);
218-
return;
219-
}
220-
221-
// API returns newest-first; reverse for chronological display
222-
const chronological = [...logs].reverse();
223-
224-
stdout.write(formatLogTable(chronological, false));
225-
226-
const hasMore = logs.length >= flags.limit;
227-
const countText = `Showing ${logs.length} log${logs.length === 1 ? "" : "s"} for trace ${traceId}.`;
228-
const tip = hasMore ? " Use --limit to show more." : "";
229-
writeFooter(stdout, `${countText}${tip}`);
212+
`Try a longer period: sentry trace logs --period 30d ${traceId}\n`,
213+
});
230214
},
231215
});

src/lib/formatters/log.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@
44
* Provides formatting utilities for displaying Sentry logs in the CLI.
55
*/
66

7-
import type { DetailedSentryLog, SentryLog } from "../../types/index.js";
7+
import type {
8+
DetailedSentryLog,
9+
SentryLog,
10+
Writer,
11+
} from "../../types/index.js";
812
import { buildTraceUrl } from "../sentry-urls.js";
13+
import { writeJson } from "./json.js";
914
import {
1015
colorTag,
1116
escapeMarkdownCell,
@@ -18,6 +23,7 @@ import {
1823
renderMarkdown,
1924
stripColorTags,
2025
} from "./markdown.js";
26+
import { writeFooter } from "./output.js";
2127
import {
2228
renderTextTable,
2329
StreamingTable,
@@ -317,3 +323,49 @@ export function formatLogDetails(
317323

318324
return renderMarkdown(lines.join("\n"));
319325
}
326+
327+
/**
328+
* Options for {@link displayTraceLogs}.
329+
*/
330+
type DisplayTraceLogsOptions = {
331+
/** Writer for output */
332+
stdout: Writer;
333+
/** Already-fetched logs (API order: newest-first) */
334+
logs: LogLike[];
335+
/** The trace ID being queried */
336+
traceId: string;
337+
/** The --limit value (used for "has more" hint) */
338+
limit: number;
339+
/** Output as JSON instead of human-readable table */
340+
asJson: boolean;
341+
/** Message to show when no logs are found */
342+
emptyMessage: string;
343+
};
344+
345+
/**
346+
* Shared display logic for trace-filtered log results.
347+
*
348+
* Handles JSON output, empty state, and human-readable table formatting.
349+
* Used by both `sentry log list --trace` and `sentry trace logs`.
350+
*/
351+
export function displayTraceLogs(options: DisplayTraceLogsOptions): void {
352+
const { stdout, logs, traceId, limit, asJson, emptyMessage } = options;
353+
354+
if (asJson) {
355+
writeJson(stdout, [...logs].reverse());
356+
return;
357+
}
358+
359+
if (logs.length === 0) {
360+
stdout.write(emptyMessage);
361+
return;
362+
}
363+
364+
const chronological = [...logs].reverse();
365+
stdout.write(formatLogTable(chronological, false));
366+
367+
const hasMore = logs.length >= limit;
368+
const countText = `Showing ${logs.length} log${logs.length === 1 ? "" : "s"} for trace ${traceId}.`;
369+
const tip = hasMore ? " Use --limit to show more." : "";
370+
writeFooter(stdout, `${countText}${tip}`);
371+
}

test/commands/trace/logs.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ describe("logsCommand.func", () => {
230230
const parsed = JSON.parse(output);
231231
expect(Array.isArray(parsed)).toBe(true);
232232
expect(parsed).toHaveLength(3);
233-
expect(parsed[0].id).toBe("log001");
233+
// displayTraceLogs reverses to chronological order for JSON output
234+
expect(parsed[0].id).toBe("log003");
234235
});
235236

236237
test("outputs empty JSON array when no logs found with --json", async () => {

0 commit comments

Comments
 (0)