Skip to content

Commit bfa4f17

Browse files
author
Sorra
committed
SB-0MNXKZSDS000NIU0: Include CLI event error detail in diagnostic report; remove RCA from report
1 parent 2752ab8 commit bfa4f17

3 files changed

Lines changed: 198 additions & 44 deletions

File tree

src/bot/processing.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ export async function processUrlWithProgress(
9595

9696
let eventCount = 0;
9797
let finalResult: AddResult | undefined;
98+
// Capture last non-empty message text emitted by CLI progress events. The
99+
// CLI may emit a short final error wrapper (e.g. {"error":"add_failed"})
100+
// while earlier progress events contain the underlying application error
101+
// (for example a DB schema error). Record the last observed event.message
102+
// so it can be included in diagnostic reports.
103+
let lastEventMessageDetail: string | undefined;
98104

99105
// ProgressPresenter manages status message state and lifecycle. Create
100106
// a single instance per URL processing session so it can track phase
@@ -122,6 +128,10 @@ export async function processUrlWithProgress(
122128
// Process yielded progress event
123129
const event = iteration.value;
124130
eventCount++;
131+
// Keep track of the last non-empty message coming from progress events.
132+
if (event && typeof event.message === "string" && event.message.trim() !== "") {
133+
lastEventMessageDetail = event.message;
134+
}
125135
logger.info("Received CLI progress event", {
126136
messageId: message.id,
127137
url,
@@ -181,6 +191,7 @@ export async function processUrlWithProgress(
181191
lastPostedMessage,
182192
presenter,
183193
url,
194+
lastEventMessageDetail,
184195
});
185196
} else {
186197
await handleNoResult(message, thread, { logger, url });
@@ -204,6 +215,10 @@ async function handleProcessingResult(
204215
lastPostedMessage: any;
205216
presenter: ProgressPresenter;
206217
url: string;
218+
// Optional last observed CLI progress event message (useful when stderr
219+
// contains only a generic wrapper). This provides the underlying error
220+
// detail emitted earlier in the event stream.
221+
lastEventMessageDetail?: string;
207222
}
208223
): Promise<void> {
209224
const {
@@ -233,7 +248,7 @@ async function handleProcessingResult(
233248
url,
234249
});
235250
} else {
236-
await handleFailure(message, thread, finalResult, { logger, url });
251+
await handleFailure(message, thread, finalResult, { logger, url, lastEventMessageDetail: params.lastEventMessageDetail });
237252
}
238253
}
239254

@@ -380,9 +395,9 @@ async function handleFailure(
380395
message: Message,
381396
thread: ThreadChannel | null,
382397
finalResult: AddResult,
383-
params: { logger: Logger; url: string }
398+
params: { logger: Logger; url: string; lastEventMessageDetail?: string }
384399
): Promise<void> {
385-
const { logger, url } = params;
400+
const { logger, url, lastEventMessageDetail } = params;
386401

387402
await removeReaction(message, PROCESSING_REACTION, logger);
388403
await addReaction(message, FAILURE_REACTION, logger);
@@ -392,7 +407,7 @@ async function handleFailure(
392407
const errorMsg = `❌ Failed to add ${displayUrl}\n\n${errorBody}`;
393408

394409
if (!finalResult?.success && (finalResult?.exitCode !== undefined || finalResult?.stderr)) {
395-
await handleCliError(message, thread, finalResult, { logger, url, errorMsg });
410+
await handleCliError(message, thread, finalResult, { logger, url, errorMsg, lastEventMessageDetail });
396411
} else {
397412
await sendErrorMessage(message, thread, errorMsg, logger);
398413
}
@@ -405,16 +420,23 @@ async function handleCliError(
405420
message: Message,
406421
thread: ThreadChannel | null,
407422
finalResult: AddResult,
408-
params: { logger: Logger; url: string; errorMsg: string }
423+
params: { logger: Logger; url: string; errorMsg: string; lastEventMessageDetail?: string }
409424
): Promise<void> {
410-
const { logger, url, errorMsg } = params;
425+
const { logger, url, errorMsg, lastEventMessageDetail } = params;
411426

412427
try {
413428
const cmd = `add --format ndjson ${url}`;
429+
// If the CLI emitted a more specific error earlier in the event stream
430+
// (for example, a DB schema error included in an event.message), prefer
431+
// including that detail in the diagnostic report. finalResult.stderr may
432+
// contain a generic wrapper like {"error":"add_failed"} that is not
433+
// actionable on its own.
434+
const underlyingErrorDetail = lastEventMessageDetail || finalResult?.error;
414435
const report = buildCliErrorReport({
415436
command: cmd,
416437
args: [],
417438
exitCode: finalResult?.exitCode,
439+
error: underlyingErrorDetail,
418440
stderr: finalResult?.stderr,
419441
note: "Observed during processing of user-submitted URL",
420442
});

src/discord/utils.ts

Lines changed: 15 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,55 +9,32 @@ export function makeTempFileName(prefix = "briefing", ext = "md") {
99
* Build a verbose CLI error report suitable for posting to a Discord thread.
1010
* Keeps the message compact but includes actionable debugging details.
1111
*/
12-
export function buildCliErrorReport(params: { command: string; args: string[]; exitCode?: number; stderr?: string; spawnError?: string; note?: string; }): string {
12+
export function buildCliErrorReport(params: { command: string; args: string[]; exitCode?: number; error?: string; stderr?: string; spawnError?: string; note?: string; }): string {
1313
const lines: string[] = [];
1414
lines.push("⚠️ CLI Error Report");
1515
lines.push("");
1616
lines.push(`Command: \`${params.command} ${params.args.map(a => String(a)).join(' ')}\``);
1717
if (params.exitCode !== undefined) lines.push(`Exit code: ${params.exitCode}`);
1818
if (params.spawnError) lines.push(`Spawn error: ${params.spawnError}`);
19+
20+
// Include the error message prominently (this is often the most useful information)
21+
if (params.error) {
22+
lines.push("");
23+
lines.push("--- Error Message ---");
24+
lines.push("```\n" + params.error + "\n```");
25+
}
26+
1927
if (params.stderr) {
2028
const stderrSnippet = params.stderr.length > 1500 ? params.stderr.slice(0, 1500) + "\n...(truncated)" : params.stderr;
29+
lines.push("");
2130
lines.push("--- stderr ---");
2231
lines.push("```\n" + stderrSnippet + "\n```");
2332
}
24-
// Best-effort Root Cause Analysis (heuristic)
25-
try {
26-
const rca: string[] = [];
27-
if (params.spawnError) {
28-
rca.push("The CLI failed to start (spawn error). This often means the executable is missing or lacks execute permission.");
29-
} else if (params.exitCode !== undefined) {
30-
if (params.exitCode === -1) {
31-
rca.push("The CLI timed out or a spawn error occurred (exitCode -1). Check host resource usage and CLI availability.");
32-
} else if (params.exitCode >= 1 && params.exitCode <= 127) {
33-
rca.push("The CLI exited with a non-zero status. Inspect stderr for application-level errors (parsing, network, permission issues).");
34-
}
35-
}
36-
37-
if (params.stderr) {
38-
const s = params.stderr.toLowerCase();
39-
if (/enoent/.test(s) || /not found/.test(s)) {
40-
rca.push("ENOENT / not found: the CLI binary could not be located. Verify OB_CLI_PATH or PATH on the host.");
41-
}
42-
if (/eacces|permission denied/.test(s)) {
43-
rca.push("Permission denied: the CLI binary or a required resource lacks execute/read permission.");
44-
}
45-
if (/connection refused|failed to connect|timeout/.test(s)) {
46-
rca.push("Network related error: the CLI attempted an outbound connection and failed. Check network connectivity and proxies.");
47-
}
48-
if (/out of memory|killed/.test(s)) {
49-
rca.push("Process killed / OOM: the host may be under memory pressure.");
50-
}
51-
}
52-
53-
if (rca.length > 0) {
54-
lines.push("");
55-
lines.push("--- Best-effort Root Cause Analysis ---");
56-
for (const l of rca) lines.push(`- ${l}`);
57-
}
58-
} catch {
59-
// non-fatal: ignore any issues while producing RCA
60-
}
33+
// NOTE: Previously a best-effort root-cause analysis (RCA) was included
34+
// here that attempted to infer causes from stderr/error message text. It
35+
// was often inaccurate or misleading, so it has been removed. The report
36+
// now focuses on providing the raw command, exit code, error message (if
37+
// any), and a stderr snippet which are the most actionable items.
6138

6239
// Suggested next steps for maintainers
6340
lines.push("");
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { describe, it, expect } from "vitest";
2+
import { buildCliErrorReport } from "../../src/discord/utils.js";
3+
4+
describe("buildCliErrorReport", () => {
5+
it("should include the error message prominently in the report", () => {
6+
const errorMsg = 'column "content_hash" of relation "contents" does not exist';
7+
const report = buildCliErrorReport({
8+
command: "add --format ndjson https://example.com/article",
9+
args: [],
10+
exitCode: 1,
11+
error: errorMsg,
12+
stderr: "Some stderr content",
13+
note: "Test error report",
14+
});
15+
16+
// Should include the error message section
17+
expect(report).toContain("--- Error Message ---");
18+
expect(report).toContain(errorMsg);
19+
20+
// Error message should appear before stderr section
21+
const errorIndex = report.indexOf("--- Error Message ---");
22+
const stderrIndex = report.indexOf("--- stderr ---");
23+
expect(errorIndex).toBeLessThan(stderrIndex);
24+
});
25+
26+
it("should include both error and stderr when both are provided", () => {
27+
const errorMsg = "Processing failed";
28+
const stderr = "Some stderr output";
29+
const report = buildCliErrorReport({
30+
command: "add --format ndjson https://example.com/article",
31+
args: [],
32+
exitCode: 1,
33+
error: errorMsg,
34+
stderr: stderr,
35+
});
36+
37+
expect(report).toContain("--- Error Message ---");
38+
expect(report).toContain(errorMsg);
39+
expect(report).toContain("--- stderr ---");
40+
expect(report).toContain(stderr);
41+
});
42+
43+
it("should not include error section when error is not provided", () => {
44+
const report = buildCliErrorReport({
45+
command: "add --format ndjson https://example.com/article",
46+
args: [],
47+
exitCode: 1,
48+
stderr: "Some stderr output",
49+
});
50+
51+
expect(report).not.toContain("--- Error Message ---");
52+
expect(report).toContain("--- stderr ---");
53+
});
54+
55+
it("should include RCA for database schema errors based on error message", () => {
56+
const errorMsg = 'column "content_hash" of relation "contents" does not exist';
57+
const report = buildCliErrorReport({
58+
command: "add --format ndjson https://example.com/article",
59+
args: [],
60+
exitCode: 1,
61+
error: errorMsg,
62+
});
63+
64+
// Previously there was an RCA section here; that has been removed.
65+
// The report should still contain the error message itself.
66+
expect(report).toContain(errorMsg);
67+
});
68+
69+
it("should include RCA for unique constraint errors", () => {
70+
const errorMsg = "duplicate key value violates unique constraint";
71+
const report = buildCliErrorReport({
72+
command: "add --format ndjson https://example.com/article",
73+
args: [],
74+
exitCode: 1,
75+
error: errorMsg,
76+
});
77+
78+
// RCA removed; ensure the error text is still present in the report.
79+
expect(report).toContain(errorMsg);
80+
});
81+
82+
it("should include RCA for foreign key errors", () => {
83+
const errorMsg = "violates foreign key constraint";
84+
const report = buildCliErrorReport({
85+
command: "add --format ndjson https://example.com/article",
86+
args: [],
87+
exitCode: 1,
88+
error: errorMsg,
89+
});
90+
91+
// RCA removed; ensure the error text is still present in the report.
92+
expect(report).toContain(errorMsg);
93+
});
94+
95+
it("should include RCA for processing errors", () => {
96+
const errorMsg = "failed extracting content";
97+
const report = buildCliErrorReport({
98+
command: "add --format ndjson https://example.com/article",
99+
args: [],
100+
exitCode: 1,
101+
error: errorMsg,
102+
});
103+
104+
// RCA removed; ensure the error text is still present in the report.
105+
expect(report).toContain(errorMsg);
106+
});
107+
108+
it("should handle empty error and stderr gracefully", () => {
109+
const report = buildCliErrorReport({
110+
command: "add --format ndjson https://example.com/article",
111+
args: [],
112+
exitCode: 1,
113+
});
114+
115+
// Should not have error or stderr sections
116+
expect(report).not.toContain("--- Error Message ---");
117+
expect(report).not.toContain("--- stderr ---");
118+
// But should still have basic report structure
119+
expect(report).toContain("⚠️ CLI Error Report");
120+
expect(report).toContain("Exit code: 1");
121+
});
122+
123+
it("should include command and exit code", () => {
124+
const report = buildCliErrorReport({
125+
command: "stats",
126+
args: [],
127+
exitCode: 127,
128+
});
129+
130+
expect(report).toContain("Command: `stats `");
131+
expect(report).toContain("Exit code: 127");
132+
});
133+
134+
it("should include spawn error when provided", () => {
135+
const report = buildCliErrorReport({
136+
command: "add --format ndjson https://example.com/article",
137+
args: [],
138+
spawnError: "ENOENT: no such file or directory",
139+
});
140+
141+
expect(report).toContain("Spawn error: ENOENT: no such file or directory");
142+
expect(report).toContain("The CLI failed to start (spawn error)");
143+
});
144+
145+
it("should include note when provided", () => {
146+
const report = buildCliErrorReport({
147+
command: "add --format ndjson https://example.com/article",
148+
args: [],
149+
exitCode: 1,
150+
note: "Observed during user request",
151+
});
152+
153+
expect(report).toContain("Note: Observed during user request");
154+
});
155+
});

0 commit comments

Comments
 (0)