Skip to content

Commit fc3e594

Browse files
committed
fix: address PR #416 review comments
Addresses 18 review comments from BYK on the generator-commands PR: #2: Use result.user = user instead of cherry-picking fields (login.ts) #3: Remove process.stdin param from runInteractiveLogin (interactive-login.ts) #4: Use return yield pattern for one-liner yields (logout.ts) #5: Make OutputConfig.json optional (defaults to true) (output.ts) #6: Convert trace/logs.ts to yield commandOutput (trace/logs.ts) #7: Move URL/QR display through yield commandOutput (trial/start.ts) #8: Rename const log → const logger, import logger as log (trial/start.ts) #9: Convert help.ts to yield commandOutput (help.ts) #11: Replace Symbol branding with class + instanceof for CommandOutput (output.ts, command.ts) #12: Remove jsonlLines/JSONL mechanism — each yield becomes one JSON line (output.ts, log/list.ts) #15-#17: Replace polling dots with spinner in interactive login (interactive-login.ts) #18: Yield individual log items in follow mode (log/list.ts) Key changes: - CommandOutput is now a class using instanceof instead of Symbol branding - JSONL support removed — streaming commands yield items individually - trace/logs.ts uses OutputConfig with human/jsonTransform instead of manual stdout - help.ts yields through output framework - trial/start.ts handlePlanTrial is now an async generator yielding display data - interactive-login.ts uses consola spinner for polling, removes stdin param - OutputConfig.json is now optional (defaults to true when object form is used) Tests updated for trace/logs and trial/start to use context stdout.
1 parent 5c4c1b1 commit fc3e594

File tree

12 files changed

+212
-267
lines changed

12 files changed

+212
-267
lines changed

src/bin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ const autoAuthMiddleware: ErrorMiddleware = async (next, args) => {
103103
: "Authentication required. Starting login flow...\n\n"
104104
);
105105

106-
const loginSuccess = await runInteractiveLogin(process.stdin);
106+
const loginSuccess = await runInteractiveLogin();
107107

108108
if (loginSuccess) {
109109
process.stderr.write("\nRetrying command...\n\n");

src/commands/auth/login.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,7 @@ export const loginCommand = buildCommand({
177177
username: user.username,
178178
name: user.name,
179179
});
180-
result.user = {
181-
name: user.name,
182-
email: user.email,
183-
username: user.username,
184-
id: user.id,
185-
};
180+
result.user = user;
186181
} catch {
187182
// Non-fatal: user info is supplementary. Token remains stored and valid.
188183
}
@@ -192,7 +187,7 @@ export const loginCommand = buildCommand({
192187
}
193188

194189
// OAuth device flow
195-
const result = await runInteractiveLogin(process.stdin, {
190+
const result = await runInteractiveLogin({
196191
timeout: flags.timeout * 1000,
197192
});
198193

src/commands/auth/logout.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,10 @@ export const logoutCommand = buildCommand({
3939
},
4040
async *func(this: SentryContext) {
4141
if (!(await isAuthenticated())) {
42-
yield commandOutput({
42+
return yield commandOutput({
4343
loggedOut: false,
4444
message: "Not currently authenticated.",
4545
});
46-
return;
4746
}
4847

4948
if (isEnvTokenActive()) {
@@ -58,10 +57,9 @@ export const logoutCommand = buildCommand({
5857
const configPath = getDbPath();
5958
await clearAuth();
6059

61-
yield commandOutput({
60+
return yield commandOutput({
6261
loggedOut: true,
6362
configPath,
6463
});
65-
return;
6664
},
6765
});

src/commands/help.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import { run } from "@stricli/core";
1010
import type { SentryContext } from "../context.js";
1111
import { buildCommand } from "../lib/command.js";
12+
import { commandOutput, stateless } from "../lib/formatters/output.js";
1213
import { printCustomHelp } from "../lib/help.js";
1314

1415
export const helpCommand = buildCommand({
@@ -18,6 +19,7 @@ export const helpCommand = buildCommand({
1819
"Display help information. Run 'sentry help' for an overview, " +
1920
"or 'sentry help <command>' for detailed help on a specific command.",
2021
},
22+
output: { json: true, human: stateless((s: string) => s) },
2123
parameters: {
2224
flags: {},
2325
positional: {
@@ -30,12 +32,10 @@ export const helpCommand = buildCommand({
3032
},
3133
},
3234
// biome-ignore lint/complexity/noBannedTypes: Stricli requires empty object for commands with no flags
33-
// biome-ignore lint/correctness/useYield: void generator — delegates to Stricli help system
3435
async *func(this: SentryContext, _flags: {}, ...commandPath: string[]) {
3536
// No args: show branded help
3637
if (commandPath.length === 0) {
37-
process.stdout.write(await printCustomHelp());
38-
return;
38+
return yield commandOutput(await printCustomHelp());
3939
}
4040

4141
// With args: re-invoke with --helpAll to show full help including hidden items

src/commands/log/list.ts

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,9 @@ import {
2222
import { filterFields } from "../../lib/formatters/json.js";
2323
import { renderInlineMarkdown } from "../../lib/formatters/markdown.js";
2424
import {
25-
type CommandOutput,
2625
commandOutput,
2726
formatFooter,
2827
type HumanRenderer,
29-
jsonlLines,
3028
} from "../../lib/formatters/output.js";
3129
import type { StreamingTable } from "../../lib/formatters/text-table.js";
3230
import {
@@ -64,12 +62,6 @@ type LogListResult = {
6462
logs: LogLike[];
6563
/** Trace ID, present for trace-filtered queries */
6664
traceId?: string;
67-
/**
68-
* When true, JSON output uses JSONL (one object per line) instead
69-
* of a JSON array. Set for follow-mode batches where output is
70-
* consumed incrementally. Does not affect human rendering.
71-
*/
72-
jsonl?: boolean;
7365
};
7466

7567
/** Maximum allowed value for --limit flag */
@@ -336,16 +328,22 @@ async function* generateFollowLogs<T extends LogLike>(
336328
}
337329

338330
/**
339-
* Consume a follow-mode generator, yielding `LogListResult` batches.
331+
* Consume a follow-mode generator, yielding each log individually.
332+
*
333+
* In JSON mode each yield becomes one JSONL line. In human mode the
334+
* stateful renderer accumulates rows into the streaming table.
335+
*
340336
* The generator returns when SIGINT fires — the wrapper's `finalize()`
341337
* callback handles closing the streaming table.
342338
*/
343339
async function* yieldFollowBatches<T extends LogLike>(
344340
generator: AsyncGenerator<T[], void, undefined>,
345341
extra?: Partial<LogListResult>
346-
): AsyncGenerator<CommandOutput<LogListResult>, void, undefined> {
342+
): AsyncGenerator<unknown, void, undefined> {
347343
for await (const batch of generator) {
348-
yield commandOutput({ logs: batch, jsonl: true, ...extra });
344+
for (const item of batch) {
345+
yield commandOutput({ logs: [item], ...extra });
346+
}
349347
}
350348
}
351349

@@ -478,25 +476,22 @@ function createLogRenderer(): HumanRenderer<LogListResult> {
478476
/**
479477
* Transform log output into the JSON shape.
480478
*
481-
* - **Single-fetch** (`jsonl` absent/false): returns a JSON array.
482-
* - **Follow mode** (`jsonl: true`): returns JSONL-wrapped objects
483-
* so the framework writes one JSON line per log entry.
479+
* Each yielded batch is written as a JSON array. In follow mode,
480+
* each batch is a short array (one poll result); in single-fetch mode
481+
* it's the full result set. Empty batches are suppressed.
484482
*/
485483
function jsonTransformLogOutput(
486484
result: LogListResult,
487485
fields?: string[]
488486
): unknown {
489-
const applyFields = (log: LogLike) =>
490-
fields && fields.length > 0 ? filterFields(log, fields) : log;
491-
492487
if (result.logs.length === 0) {
493-
// Follow mode: suppress empty batches (no JSONL output)
494-
// Single-fetch: return empty array for valid JSON
495-
return result.jsonl ? undefined : [];
488+
return;
496489
}
497490

498-
const mapped = result.logs.map(applyFields);
499-
return result.jsonl ? jsonlLines(mapped) : mapped;
491+
const applyFields = (log: LogLike) =>
492+
fields && fields.length > 0 ? filterFields(log, fields) : log;
493+
494+
return result.logs.map(applyFields);
500495
}
501496

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

src/commands/trace/logs.ts

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,13 @@ 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 { formatTraceLogs } from "../../lib/formatters/index.js";
13+
import { filterFields } from "../../lib/formatters/json.js";
14+
import { formatLogTable } from "../../lib/formatters/log.js";
15+
import {
16+
commandOutput,
17+
formatFooter,
18+
stateless,
19+
} from "../../lib/formatters/output.js";
1420
import {
1521
applyFreshFlag,
1622
FRESH_ALIASES,
@@ -30,6 +36,34 @@ type LogsFlags = {
3036
readonly fields?: string[];
3137
};
3238

39+
/** Minimal log shape shared with the formatters. */
40+
type LogLike = {
41+
timestamp: string;
42+
severity?: string | null;
43+
message?: string | null;
44+
trace?: string | null;
45+
};
46+
47+
/** Data yielded by the trace logs command. */
48+
type TraceLogsData = {
49+
logs: LogLike[];
50+
traceId: string;
51+
limit: number;
52+
};
53+
54+
/** Format trace log results as human-readable table output. */
55+
function formatTraceLogsHuman(data: TraceLogsData): string {
56+
if (data.logs.length === 0) {
57+
return "";
58+
}
59+
const parts = [formatLogTable(data.logs, false)];
60+
const hasMore = data.logs.length >= data.limit;
61+
const countText = `Showing ${data.logs.length} log${data.logs.length === 1 ? "" : "s"} for trace ${data.traceId}.`;
62+
const tip = hasMore ? " Use --limit to show more." : "";
63+
parts.push(formatFooter(`${countText}${tip}`));
64+
return parts.join("");
65+
}
66+
3367
/** Maximum allowed value for --limit flag */
3468
const MAX_LIMIT = 1000;
3569

@@ -132,7 +166,16 @@ export const logsCommand = buildCommand({
132166
" sentry trace logs --period 7d abc123def456abc123def456abc123de\n" +
133167
" sentry trace logs --json abc123def456abc123def456abc123de",
134168
},
135-
output: "json",
169+
output: {
170+
json: true,
171+
human: stateless(formatTraceLogsHuman),
172+
jsonTransform: (data: TraceLogsData, fields?: string[]) => {
173+
if (fields && fields.length > 0) {
174+
return data.logs.map((entry) => filterFields(entry, fields));
175+
}
176+
return data.logs;
177+
},
178+
},
136179
parameters: {
137180
positional: {
138181
kind: "array",
@@ -176,7 +219,6 @@ export const logsCommand = buildCommand({
176219
q: "query",
177220
},
178221
},
179-
// biome-ignore lint/correctness/useYield: void generator — early returns for web mode
180222
async *func(this: SentryContext, flags: LogsFlags, ...args: string[]) {
181223
applyFreshFlag(flags);
182224
const { cwd, setContext } = this;
@@ -206,17 +248,21 @@ export const logsCommand = buildCommand({
206248
query: flags.query,
207249
});
208250

209-
process.stdout.write(
210-
formatTraceLogs({
211-
logs,
212-
traceId,
213-
limit: flags.limit,
214-
asJson: flags.json,
215-
fields: flags.fields,
216-
emptyMessage:
251+
// Reverse to chronological order (API returns newest-first)
252+
const chronological = [...logs].reverse();
253+
254+
yield commandOutput({
255+
logs: chronological,
256+
traceId,
257+
limit: flags.limit,
258+
});
259+
260+
if (logs.length === 0) {
261+
return {
262+
hint:
217263
`No logs found for trace ${traceId} in the last ${flags.period}.\n\n` +
218-
`Try a longer period: sentry trace logs --period 30d ${traceId}\n`,
219-
})
220-
);
264+
`Try a longer period: sentry trace logs --period 30d ${traceId}`,
265+
};
266+
}
221267
},
222268
});

0 commit comments

Comments
 (0)