fix(tools): classify expected input and timeout errors, not bugs (#539)

crop/merge-pdf/csv bad input -> ToolInputError/InputValidationError (expected, 4xx); ffmpeg timeout -> operational SafeError. Internal v2-only guards stay plain Errors.
This commit is contained in:
SnapOtter
2026-07-16 19:26:18 +08:00
committed by GitHub
parent 55e1e95f20
commit 39b89b9fbd
6 changed files with 62 additions and 13 deletions
+3 -2
View File
@@ -1,6 +1,7 @@
import type { FastifyInstance } from "fastify";
import Papa from "papaparse";
import { z } from "zod";
import { InputValidationError } from "../../modality/contract.js";
import { createToolRoute } from "../tool-factory.js";
const settingsSchema = z.object({
@@ -31,7 +32,7 @@ export function registerCsvExcel(app: FastifyInstance) {
await workbook.xlsx.load(input.buffer as unknown as ArrayBuffer);
const ws = workbook.worksheets[settings.sheet - 1];
if (!ws) {
throw new Error(
throw new InputValidationError(
`Worksheet ${settings.sheet} not found (workbook has ${workbook.worksheets.length} sheets)`,
);
}
@@ -59,7 +60,7 @@ export function registerCsvExcel(app: FastifyInstance) {
skipEmptyLines: true,
});
if (parsed.errors.length > 0) {
throw new Error(`CSV parse failed: ${parsed.errors[0].message}`);
throw new InputValidationError(`CSV parse failed: ${parsed.errors[0].message}`);
}
const workbook = new ExcelJS.Workbook();
const ws = workbook.addWorksheet("Sheet1");
+7 -4
View File
@@ -1,6 +1,7 @@
import type { FastifyInstance } from "fastify";
import Papa from "papaparse";
import { z } from "zod";
import { InputValidationError } from "../../modality/contract.js";
import { createToolRoute } from "../tool-factory.js";
const settingsSchema = z.object({
@@ -26,13 +27,15 @@ export function registerCsvJson(app: FastifyInstance) {
data = JSON.parse(input.buffer.toString("utf8"));
} catch (err: unknown) {
const msg = err instanceof Error ? err.message : String(err);
throw new Error(`Not valid JSON: ${msg.split("\n")[0]}`);
throw new InputValidationError(`Not valid JSON: ${msg.split("\n")[0]}`);
}
if (!Array.isArray(data)) {
throw new Error("JSON input must be an array of objects to convert to CSV");
throw new InputValidationError(
"JSON input must be an array of objects to convert to CSV",
);
}
if (data.some((r) => r === null || typeof r !== "object" || Array.isArray(r))) {
throw new Error("JSON array elements must be objects to convert to CSV");
throw new InputValidationError("JSON array elements must be objects to convert to CSV");
}
// Flatten nested objects/arrays to JSON strings (Papa would otherwise emit
// "[object Object]"), and pass the union of all keys so columns appearing
@@ -58,7 +61,7 @@ export function registerCsvJson(app: FastifyInstance) {
skipEmptyLines: true,
});
if (parsed.errors.length > 0) {
throw new Error(`CSV parse failed: ${parsed.errors[0].message}`);
throw new InputValidationError(`CSV parse failed: ${parsed.errors[0].message}`);
}
const json = JSON.stringify(parsed.data, null, settings.pretty ? 2 : 0);
return {
+2 -1
View File
@@ -3,6 +3,7 @@ import { join } from "node:path";
import { qpdfMerge } from "@snapotter/doc-engine";
import type { FastifyInstance } from "fastify";
import { z } from "zod";
import { InputValidationError } from "../../modality/contract.js";
import { createToolRoute } from "../tool-factory.js";
const settingsSchema = z.object({});
@@ -17,7 +18,7 @@ export function registerMergePdf(app: FastifyInstance) {
},
processV2: async (ctx) => {
if (ctx.inputs.length < 2) {
throw new Error("Merging needs at least two PDFs");
throw new InputValidationError("Merging needs at least two PDFs");
}
ctx.report(10, "Staging");
const paths: string[] = [];
+5 -4
View File
@@ -1,3 +1,4 @@
import { ToolInputError } from "@snapotter/shared";
import type { CropOptions, Sharp } from "../types.js";
export async function crop(image: Sharp, options: CropOptions): Promise<Sharp> {
@@ -23,19 +24,19 @@ export async function crop(image: Sharp, options: CropOptions): Promise<Sharp> {
}
if (width <= 0 || height <= 0) {
throw new Error("Crop width and height must be greater than 0");
throw new ToolInputError("Crop width and height must be greater than 0");
}
if (left < 0 || top < 0) {
throw new Error("Crop left and top must be non-negative");
throw new ToolInputError("Crop left and top must be non-negative");
}
if (left + width > imgWidth) {
throw new Error(
throw new ToolInputError(
`Crop region exceeds image width: left(${left}) + width(${width}) > ${imgWidth}`,
);
}
if (top + height > imgHeight) {
throw new Error(
throw new ToolInputError(
`Crop region exceeds image height: top(${top}) + height(${height}) > ${imgHeight}`,
);
}
+2 -2
View File
@@ -1,5 +1,5 @@
import { spawn } from "node:child_process";
import { markToolInputError } from "@snapotter/shared";
import { markToolInputError, SafeError } from "@snapotter/shared";
import { resolveFfmpeg } from "./binaries.js";
import { type FfmpegProgress, parseProgressBlock } from "./progress.js";
@@ -48,7 +48,7 @@ export async function runFfmpeg(args: string[], opts: RunFfmpegOptions = {}): Pr
const timeoutMs = opts.timeoutMs;
const timer = timeoutMs
? setTimeout(() => {
fail(new Error(`ffmpeg timed out after ${Math.round(timeoutMs / 1000)}s`));
fail(new SafeError("ffmpeg timed out", { kind: "operational", code: "timeout" }));
}, timeoutMs)
: undefined;
const onAbort = () => fail(new Error("Canceled"));
@@ -0,0 +1,43 @@
import { isToolInputError } from "@snapotter/shared";
import sharp from "sharp";
import { beforeAll, describe, expect, it } from "vitest";
import { crop } from "../../../packages/image-engine/src/operations/crop.js";
let png: Buffer;
beforeAll(async () => {
png = await sharp({
create: { width: 4, height: 4, channels: 3, background: { r: 0, g: 0, b: 0 } },
})
.png()
.toBuffer();
});
describe("crop validation", () => {
it("rejects an out-of-bounds crop as a ToolInputError (expected input, not a bug)", async () => {
let caught: unknown;
try {
await crop(sharp(png), { left: 0, top: 0, width: 100, height: 100, unit: "px" });
} catch (e) {
caught = e;
}
expect(isToolInputError(caught)).toBe(true);
expect((caught as Error).message).toContain("exceeds image width");
});
it("rejects a non-positive crop size as a ToolInputError", async () => {
let caught: unknown;
try {
await crop(sharp(png), { left: 0, top: 0, width: 0, height: 4, unit: "px" });
} catch (e) {
caught = e;
}
expect(isToolInputError(caught)).toBe(true);
});
it("crops a valid region without error", async () => {
const result = await crop(sharp(png), { left: 0, top: 0, width: 2, height: 2, unit: "px" });
const meta = await sharp(await result.png().toBuffer()).metadata();
expect(meta.width).toBe(2);
expect(meta.height).toBe(2);
});
});