mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
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:
@@ -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) });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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;
|
||||||
|
}
|
||||||
|
|||||||
@@ -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 {
|
||||||
|
|||||||
Reference in New Issue
Block a user