fix(api): return user-safe processing errors, keep raw stderr in logs

Add friendlyError() which collapses raw external-tool failure output (ffmpeg/ffprobe/LibreOffice/qpdf/etc.) into one generic sentence while preserving intentional validation messages and scrubbing internal paths. Apply it at every client-facing error surface in the tool factory and job worker (sync 422, async SSE, pipeline + batch finalize). The full error is still recorded server-side via request.log.error / logger.error and telemetry.
This commit is contained in:
SnapOtter
2026-06-17 14:22:16 +08:00
parent e96c314ab9
commit 4af4bfa8eb
3 changed files with 42 additions and 10 deletions
+14 -8
View File
@@ -30,7 +30,7 @@ import { eq } from "drizzle-orm";
import { env } from "../config.js"; import { env } from "../config.js";
import { db, schema } from "../db/index.js"; import { db, schema } from "../db/index.js";
import { resolveConcurrency } from "../lib/env.js"; import { resolveConcurrency } from "../lib/env.js";
import { stripInternalPaths } from "../lib/errors.js"; import { friendlyError } from "../lib/errors.js";
import { logger } from "../lib/logger.js"; import { logger } from "../lib/logger.js";
import { jobDuration, jobsTotal } from "../lib/metrics.js"; import { jobDuration, jobsTotal } from "../lib/metrics.js";
import { getObjectBuffer, putObject } from "../lib/object-storage.js"; import { getObjectBuffer, putObject } from "../lib/object-storage.js";
@@ -340,6 +340,12 @@ async function processToolJob(job: Job<ToolJobData>): Promise<ToolJobResult> {
? `Timed out after ${Math.round(timeoutMs / 1000)}s` ? `Timed out after ${Math.round(timeoutMs / 1000)}s`
: errorMessage; : errorMessage;
// Keep the full error (incl. raw tool stderr) in server logs; clients only
// ever see friendlyError(finalError).
if (!isCanceled && !isTimeout) {
logger.error({ err, jobId, toolId: data.toolId }, "tool job failed");
}
// Record error on the OTel span // Record error on the OTel span
if (span) { if (span) {
span.setStatus({ code: SpanStatusCode.ERROR, message: finalError }); span.setStatus({ code: SpanStatusCode.ERROR, message: finalError });
@@ -366,7 +372,7 @@ async function processToolJob(job: Job<ToolJobData>): Promise<ToolJobResult> {
status: isCanceled ? "canceled" : "failed", status: isCanceled ? "canceled" : "failed",
completedAt: new Date(), completedAt: new Date(),
durationMs, durationMs,
error: { message: finalError }, error: { message: friendlyError(finalError) },
}) })
.where(eq(schema.jobs.id, jobId)) .where(eq(schema.jobs.id, jobId))
.catch(() => {}); .catch(() => {});
@@ -387,7 +393,7 @@ async function processToolJob(job: Job<ToolJobData>): Promise<ToolJobResult> {
jobId: progressJobId, jobId: progressJobId,
phase: "failed", phase: "failed",
percent: 0, percent: 0,
error: stripInternalPaths(finalError), error: friendlyError(finalError),
}); });
} }
} }
@@ -441,7 +447,7 @@ async function processPipelineStep(job: Job<ToolJobData>): Promise<ToolJobResult
if (!prevRow || prevRow.status === "failed" || !prevRow.outputRefs?.[0]) { if (!prevRow || prevRow.status === "failed" || !prevRow.outputRefs?.[0]) {
// Previous step failed -- propagate the error without processing. // Previous step failed -- propagate the error without processing.
const prevError = stripInternalPaths( const prevError = friendlyError(
prevRow?.status === "failed" prevRow?.status === "failed"
? ((prevRow.error as { message?: string } | null)?.message ?? "Processing failed") ? ((prevRow.error as { message?: string } | null)?.message ?? "Processing failed")
: "Previous step has no output", : "Previous step has no output",
@@ -481,7 +487,7 @@ async function processPipelineStep(job: Job<ToolJobData>): Promise<ToolJobResult
// Step failed -- return failure marker. processToolJob already // Step failed -- return failure marker. processToolJob already
// updated the DB row to "failed" and emitted a terminal event // updated the DB row to "failed" and emitted a terminal event
// on the step's own progress channel. // on the step's own progress channel.
const errorMsg = stripInternalPaths(err instanceof Error ? err.message : String(err)); const errorMsg = friendlyError(err instanceof Error ? err.message : String(err));
return { return {
outputRefs: [], outputRefs: [],
filename: data.filename, filename: data.filename,
@@ -580,7 +586,7 @@ async function processPipelineFinalize(job: Job<ToolJobData>): Promise<ToolJobRe
// ── Failure path ──────────────────────────────────────────── // ── Failure path ────────────────────────────────────────────
if (failedAtStep !== null) { if (failedAtStep !== null) {
const errorMsg = stripInternalPaths(`Step ${failedAtStep + 1}: ${failError}`); const errorMsg = `Step ${failedAtStep + 1}: ${friendlyError(failError)}`;
await db await db
.update(schema.jobs) .update(schema.jobs)
@@ -696,7 +702,7 @@ async function processBatchChild(job: Job<ToolJobData>): Promise<ToolJobResult>
await recordChildOutcome(parentId, totalFiles, job.data.filename); await recordChildOutcome(parentId, totalFiles, job.data.filename);
return result; return result;
} catch (err) { } catch (err) {
const error = stripInternalPaths(err instanceof Error ? err.message : String(err)); const error = friendlyError(err instanceof Error ? err.message : String(err));
await recordChildOutcome(parentId, totalFiles, job.data.filename, error); await recordChildOutcome(parentId, totalFiles, job.data.filename, error);
// Return a completed job with a failure marker so the parent runs. // Return a completed job with a failure marker so the parent runs.
return { return {
@@ -747,7 +753,7 @@ async function processBatchFinalize(job: Job<ToolJobData>): Promise<ToolJobResul
} else { } else {
const errorMsg = (row.error as { message?: string } | null)?.message ?? "Processing failed"; const errorMsg = (row.error as { message?: string } | null)?.message ?? "Processing failed";
const inputFilename = row.inputRefs?.[0]?.split("/").pop() ?? `file-${i}`; const inputFilename = row.inputRefs?.[0]?.split("/").pop() ?? `file-${i}`;
manifest.push({ index: i, filename: inputFilename, error: stripInternalPaths(errorMsg) }); manifest.push({ index: i, filename: inputFilename, error: friendlyError(errorMsg) });
} }
} }
+23
View File
@@ -13,3 +13,26 @@ export function formatZodErrors(issues: ZodIssue[]): string {
export function stripInternalPaths(message: string): string { export function stripInternalPaths(message: string): string {
return message.replace(/\/(tmp|data|app|opt|home|workspace)\b[^\s'")}]*/g, "[internal]"); return message.replace(/\/(tmp|data|app|opt|home|workspace)\b[^\s'")}]*/g, "[internal]");
} }
/**
* Markers of a raw external-tool failure dump (ffmpeg/ffprobe/libvpx/x264/
* LibreOffice/ghostscript/qpdf/python traceback). These messages are meant for
* server logs, never for end users.
*/
const RAW_TOOL_FAILURE =
/ffmpeg exited|ffprobe|conversion failed|libvpx|x26[45] \[|stream mapping|pixel format|could not open encoder|segmentation fault|core dump|traceback \(most recent|gs: |libreoffice|qpdf:/i;
/**
* Produce a user-safe error detail. Intentional validation messages (short,
* single-line) pass through unchanged after path scrubbing; raw tool-runner
* dumps collapse to one generic sentence. The full error is still recorded in
* server logs and telemetry by the caller -- only the client-facing string is
* sanitized. Idempotent, so it is safe to apply at every error surface.
*/
export function friendlyError(message: string): string {
const cleaned = stripInternalPaths(message);
if (RAW_TOOL_FAILURE.test(cleaned) || cleaned.length > 280 || cleaned.split("\n").length > 3) {
return "Processing failed. The file may be in an unsupported or corrupted format.";
}
return cleaned;
}
+5 -2
View File
@@ -10,7 +10,7 @@ import { env } from "../config.js";
import { db, schema } from "../db/index.js"; import { db, schema } from "../db/index.js";
import { enqueueToolJob, waitForJob } from "../jobs/enqueue.js"; import { enqueueToolJob, waitForJob } from "../jobs/enqueue.js";
import { trackEvent } from "../lib/analytics.js"; import { trackEvent } from "../lib/analytics.js";
import { formatZodErrors, stripInternalPaths } from "../lib/errors.js"; import { formatZodErrors, friendlyError, stripInternalPaths } from "../lib/errors.js";
import { isToolInstalled } from "../lib/feature-status.js"; import { isToolInstalled } from "../lib/feature-status.js";
import { getObjectBuffer, putObject } from "../lib/object-storage.js"; import { getObjectBuffer, putObject } from "../lib/object-storage.js";
import { resolveToolPool, shouldSkipSyncWindow } from "../lib/pool.js"; import { resolveToolPool, shouldSkipSyncWindow } from "../lib/pool.js";
@@ -587,9 +587,12 @@ export function createToolRoute<T>(app: FastifyInstance, config: ToolRouteConfig
error_code: err instanceof Error ? err.constructor.name : "UnknownError", error_code: err instanceof Error ? err.constructor.name : "UnknownError",
error_message: err instanceof Error ? err.message.slice(0, 200) : "Processing failed", error_message: err instanceof Error ? err.message.slice(0, 200) : "Processing failed",
}); });
// Keep the full error (incl. raw ffmpeg/tool stderr) in server logs,
// but return only a user-safe detail to the client.
request.log.error({ err, toolId: config.toolId }, "tool processing failed");
return reply.status(422).send({ return reply.status(422).send({
error: "Processing failed", error: "Processing failed",
details: stripInternalPaths(err instanceof Error ? err.message : String(err)), details: friendlyError(err instanceof Error ? err.message : String(err)),
}); });
} }
} finally { } finally {