Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 5 additions & 12 deletions src/presentation/output/error_output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,13 @@
// You should have received a copy of the GNU Affero General Public License
// along with Swamp. If not, see <https://www.gnu.org/licenses/>.

import { ValidationError } from "@cliffy/command";
import { getSwampLogger } from "../../infrastructure/logging/logger.ts";
import { UserError } from "../../domain/errors.ts";
import type { OutputMode } from "./output.ts";

const logger = getSwampLogger(["error"]);

/**
* Returns true if the error is a Cliffy missing-argument error.
* These are plain Error objects with the message pattern "Missing argument(s): ..."
* and should be rendered without a stack trace.
*/
function isCliffyMissingArgError(err: Error): boolean {
return err.message.startsWith("Missing argument(s):");
}

/**
* Builds the JSON error object for structured output.
* Format: { error: string, stack?: string }
Expand All @@ -40,7 +32,8 @@ function isCliffyMissingArgError(err: Error): boolean {
export function buildErrorJson(err: Error): Record<string, string> {
const data: Record<string, string> = { error: err.message };
if (
!(err instanceof UserError) && !isCliffyMissingArgError(err) && err.stack
!(err instanceof UserError) && !(err instanceof ValidationError) &&
err.stack
) {
const stackLines = err.stack.split("\n").filter((line) =>
line.trim().startsWith("at ")
Expand All @@ -54,7 +47,7 @@ export function buildErrorJson(err: Error): Record<string, string> {

/**
* Renders an error via LogTape at fatal level.
* UserError instances and Cliffy missing-argument errors log just the message (no stack trace).
* UserError instances and Cliffy ValidationErrors log just the message (no stack trace).
* Other errors log the full Error object (including stack trace via Deno.inspect).
*
* In JSON mode, also writes the error as JSON to stdout so pipe consumers
Expand All @@ -68,7 +61,7 @@ export function renderError(error: unknown, outputMode?: OutputMode): void {
console.log(JSON.stringify(buildErrorJson(err), null, 2));
}

if (err instanceof UserError || isCliffyMissingArgError(err)) {
if (err instanceof UserError || err instanceof ValidationError) {
logger.fatal("Error: {message}", { message: err.message });
} else {
logger.fatal("{error}", { error: err });
Expand Down
80 changes: 76 additions & 4 deletions src/presentation/output/error_output_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
// along with Swamp. If not, see <https://www.gnu.org/licenses/>.

import { assertEquals, assertStringIncludes } from "@std/assert";
import { ValidationError } from "@cliffy/command";
import { initializeLogging } from "../../infrastructure/logging/logger.ts";
import { buildErrorJson, renderError } from "./error_output.ts";
import { UserError } from "../../domain/errors.ts";
Expand Down Expand Up @@ -73,13 +74,13 @@ Deno.test("renderError wraps non-Error values in Error", () => {
}
});

Deno.test("renderError logs Cliffy missing argument error without stack trace", () => {
Deno.test("renderError logs Cliffy ValidationError missing argument without stack trace", () => {
const logs: string[] = [];
const originalError = console.error;
console.error = (...args: unknown[]) => logs.push(args.join(" "));

try {
const error = new Error("Missing argument(s): extension");
const error = new ValidationError("Missing argument(s): extension");
renderError(error);

assertEquals(logs.length, 1);
Expand All @@ -91,6 +92,75 @@ Deno.test("renderError logs Cliffy missing argument error without stack trace",
}
});

Deno.test("renderError suppresses Cliffy Command dump on ValidationError (issue #171)", () => {
const logs: string[] = [];
const originalError = console.error;
console.error = (...args: unknown[]) => logs.push(args.join(" "));

try {
const error = new ValidationError(
'Unknown option "--inputs". Did you mean option "--input"?',
);
// Mimic Cliffy's real failure: ValidationError carries a `cmd` payload
// that points at the parsed Command tree (with circular refs).
const fakeCmd: Record<string, unknown> = {
settings: { name: "swamp", description: "AI Native Automation CLI" },
};
fakeCmd.cmd = fakeCmd;
Object.assign(error, { cmd: fakeCmd, exitCode: 2 });

renderError(error);

assertEquals(logs.length, 1);
assertStringIncludes(
logs[0],
'Unknown option "--inputs". Did you mean option "--input"?',
);
// The bug was a 300-line dump of Cliffy internals — none of these markers
// should leak into user-facing output.
assertEquals(logs[0].includes("Command {"), false);
assertEquals(logs[0].includes("settings:"), false);
assertEquals(logs[0].includes("[Circular"), false);
assertEquals(logs[0].includes(" at "), false);
} finally {
console.error = originalError;
}
});

Deno.test("renderError: json mode emits clean JSON for ValidationError (issue #171)", () => {
const stdoutLogs: string[] = [];
const originalLog = console.log;
const originalError = console.error;
console.log = (...args: unknown[]) => stdoutLogs.push(args.join(" "));
console.error = () => {};

try {
const error = new ValidationError(
'Unknown option "--inputs". Did you mean option "--input"?',
);
const fakeCmd: Record<string, unknown> = { settings: { name: "swamp" } };
fakeCmd.cmd = fakeCmd;
Object.assign(error, { cmd: fakeCmd, exitCode: 2 });

renderError(error, "json");

assertEquals(stdoutLogs.length, 1);
const parsed = JSON.parse(stdoutLogs[0]);
assertEquals(
parsed.error,
'Unknown option "--inputs". Did you mean option "--input"?',
);
// ValidationError should never produce a stack in JSON output.
assertEquals(parsed.stack, undefined);
// And the raw payload must not leak.
assertEquals(stdoutLogs[0].includes("Command {"), false);
assertEquals(stdoutLogs[0].includes("[Circular"), false);
} finally {
console.log = originalLog;
console.error = originalError;
}
});

Deno.test("renderError uses fatal level for all errors", () => {
const logs: string[] = [];
const originalError = console.error;
Expand Down Expand Up @@ -186,8 +256,10 @@ Deno.test("buildErrorJson: system Error includes stack", () => {
assertStringIncludes(result.stack!, "at ");
});

Deno.test("buildErrorJson: Cliffy missing arg error has no stack", () => {
const result = buildErrorJson(new Error("Missing argument(s): extension"));
Deno.test("buildErrorJson: Cliffy ValidationError has no stack", () => {
const result = buildErrorJson(
new ValidationError("Missing argument(s): extension"),
);
assertEquals(result.error, "Missing argument(s): extension");
assertEquals(result.stack, undefined);
});
Loading