mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix(telemetry): surface AI sidecar and DOMException failure reasons in Sentry (#612)
AI sidecar failures reached Sentry as 'Error: Error': the scrubber type-onlys plain Errors and the tool wrappers threw them from result.error. The bridge now exports toSidecarError(), wrapping the sidecar reason in a SafeError (memory-allocation text classifies as operational, the rest as bug); all 14 wrappers use it, plus the dispatcher crash/stdin/spawn rejection paths and parseStdoutJson. toBgRemovalError from #535 delegates to the shared helper. On the web side, DOMExceptions report their specific name via err.name, so the NATIVE_ERRORS allowlist dropped the whole family's browser-authored messages. It now carries the full WebIDL DOMException name table; messages still pass through url/path redaction. Bridge-mocking test files switched to importOriginal passthrough mocks.
This commit is contained in:
@@ -6,10 +6,14 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
// `./bridge.js` import receives these stubs.
|
||||
const runPythonWithProgress = vi.fn();
|
||||
const parseStdoutJson = vi.fn();
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
runPythonWithProgress: (...args: unknown[]) => runPythonWithProgress(...args),
|
||||
parseStdoutJson: (...args: unknown[]) => parseStdoutJson(...args),
|
||||
}));
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>();
|
||||
return {
|
||||
...actual,
|
||||
runPythonWithProgress: (...args: unknown[]) => runPythonWithProgress(...args),
|
||||
parseStdoutJson: (...args: unknown[]) => parseStdoutJson(...args),
|
||||
};
|
||||
});
|
||||
|
||||
import { isSafeMessageError, SafeError } from "@snapotter/shared";
|
||||
import { removeBackground } from "../../../packages/ai/src/background-removal.js";
|
||||
|
||||
@@ -17,7 +17,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
unlink: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
import { type ChildProcess, spawn } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { Writable } from "node:stream";
|
||||
import { isSafeMessageError, type SafeError } from "@snapotter/shared";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// Mock child_process.spawn before importing the bridge module
|
||||
vi.mock("node:child_process", () => ({
|
||||
spawn: vi.fn(),
|
||||
}));
|
||||
|
||||
function createMockProcess(): {
|
||||
process: ChildProcess;
|
||||
stdout: EventEmitter;
|
||||
stderr: EventEmitter;
|
||||
emitEvent: (event: string, ...args: unknown[]) => void;
|
||||
} {
|
||||
const stdin = new Writable({
|
||||
write(_chunk, _encoding, callback) {
|
||||
callback();
|
||||
},
|
||||
});
|
||||
const stdout = new EventEmitter();
|
||||
const stderr = new EventEmitter();
|
||||
const proc = new EventEmitter() as unknown as ChildProcess;
|
||||
Object.assign(proc, {
|
||||
stdin,
|
||||
stdout,
|
||||
stderr,
|
||||
pid: 12345,
|
||||
killed: false,
|
||||
kill: vi.fn(() => {
|
||||
(proc as { killed: boolean }).killed = true;
|
||||
return true;
|
||||
}),
|
||||
});
|
||||
return {
|
||||
process: proc,
|
||||
stdout,
|
||||
stderr,
|
||||
emitEvent: (event: string, ...args: unknown[]) => proc.emit(event, ...args),
|
||||
};
|
||||
}
|
||||
|
||||
// Every rejection the bridge hands to a tool wrapper must be a SafeError:
|
||||
// plain Errors get reduced to "Error: Error" by the API's Sentry scrubber
|
||||
// (NODE-24, NODE-1R), losing the failure reason.
|
||||
describe("bridge rejections are SafeErrors", () => {
|
||||
let bridge: typeof import("../../../packages/ai/src/bridge.js");
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.resetModules();
|
||||
vi.mocked(spawn).mockReset();
|
||||
bridge = await import("../../../packages/ai/src/bridge.js");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("parseStdoutJson throws a SafeError (bug) when the sidecar returns no JSON", () => {
|
||||
let caught: unknown;
|
||||
try {
|
||||
bridge.parseStdoutJson("not json at all");
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(isSafeMessageError(caught)).toBe(true);
|
||||
expect((caught as SafeError).kind).toBe("bug");
|
||||
expect((caught as SafeError).message).toBe("No JSON response from Python script");
|
||||
});
|
||||
|
||||
it("per-request spawn failure (non-ENOENT) rejects with an operational SafeError", async () => {
|
||||
const mockDisp = createMockProcess();
|
||||
const mockPerReq = createMockProcess();
|
||||
let callCount = 0;
|
||||
vi.mocked(spawn).mockImplementation(() => {
|
||||
callCount++;
|
||||
if (callCount === 1) return mockDisp.process;
|
||||
return mockPerReq.process;
|
||||
});
|
||||
|
||||
const promise = bridge.runPythonWithProgress("test.py", []);
|
||||
|
||||
// Kill the dispatcher attempt so the per-request fallback runs.
|
||||
const enoent = new Error("ENOENT") as NodeJS.ErrnoException;
|
||||
enoent.code = "ENOENT";
|
||||
mockDisp.emitEvent("error", enoent);
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
const eacces = new Error("spawn python3 EACCES") as NodeJS.ErrnoException;
|
||||
eacces.code = "EACCES";
|
||||
mockPerReq.emitEvent("error", eacces);
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
await promise;
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(isSafeMessageError(caught)).toBe(true);
|
||||
expect((caught as SafeError).kind).toBe("operational");
|
||||
expect((caught as SafeError).message).toBe("spawn python3 EACCES");
|
||||
});
|
||||
|
||||
it("a dispatcher process error mid-request rejects pending requests with an operational SafeError", async () => {
|
||||
const mockDisp = createMockProcess();
|
||||
vi.mocked(spawn).mockReturnValue(mockDisp.process);
|
||||
|
||||
// Let the dispatcher come up, then issue a request against it.
|
||||
const initPromise = bridge.initDispatcher(1_000);
|
||||
mockDisp.stderr.emit("data", Buffer.from('{"ready": true, "gpu": false}\n'));
|
||||
await initPromise;
|
||||
|
||||
const promise = bridge.runPythonWithProgress("test.py", []);
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
// A process-level error whose message matches no retry rule propagates
|
||||
// straight to the caller, so it must already be a SafeError.
|
||||
const eagain = new Error("spawn EAGAIN") as NodeJS.ErrnoException;
|
||||
eagain.code = "EAGAIN";
|
||||
mockDisp.emitEvent("error", eagain);
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
await promise;
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(isSafeMessageError(caught)).toBe(true);
|
||||
expect((caught as SafeError).kind).toBe("operational");
|
||||
});
|
||||
});
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -15,7 +15,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
unlink: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
unlink: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -15,7 +15,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -14,7 +14,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
import { tmpdir } from "node:os";
|
||||
import { isSafeMessageError, SafeError } from "@snapotter/shared";
|
||||
import sharp from "sharp";
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// Mock only the process-spawning entry point; toSidecarError and
|
||||
// parseStdoutJson stay real so the wrappers exercise the actual wrap logic.
|
||||
const runPythonWithProgress = vi.fn();
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>();
|
||||
return {
|
||||
...actual,
|
||||
runPythonWithProgress: (...args: unknown[]) => runPythonWithProgress(...args),
|
||||
};
|
||||
});
|
||||
|
||||
import { toSidecarError } from "../../../packages/ai/src/bridge.js";
|
||||
import { transcribeAudio } from "../../../packages/ai/src/transcription.js";
|
||||
import { upscale } from "../../../packages/ai/src/upscaling.js";
|
||||
|
||||
beforeEach(() => {
|
||||
runPythonWithProgress.mockReset();
|
||||
});
|
||||
|
||||
describe("toSidecarError", () => {
|
||||
it("wraps a sidecar reason string in a SafeError bug that keeps the reason as message", () => {
|
||||
const err = toSidecarError("rembg model load failed", "Background removal failed");
|
||||
expect(isSafeMessageError(err)).toBe(true);
|
||||
expect(err.message).toBe("rembg model load failed");
|
||||
expect((err as SafeError).kind).toBe("bug");
|
||||
});
|
||||
|
||||
it("classifies memory-allocation reasons as operational (environment, not our bug)", () => {
|
||||
for (const reason of [
|
||||
"CUDA out of memory",
|
||||
"Failed to allocate memory for requested buffer",
|
||||
"CUBLAS_STATUS_ALLOC_FAILED",
|
||||
"std::bad_alloc",
|
||||
]) {
|
||||
const err = toSidecarError(reason, "Upscaling failed") as SafeError;
|
||||
expect(err.kind).toBe("operational");
|
||||
}
|
||||
});
|
||||
|
||||
it("falls back to the constant tool message when the reason is empty", () => {
|
||||
for (const reason of [undefined, null, ""]) {
|
||||
const err = toSidecarError(reason, "Upscaling failed");
|
||||
expect(isSafeMessageError(err)).toBe(true);
|
||||
expect(err.message).toBe("Upscaling failed");
|
||||
}
|
||||
});
|
||||
|
||||
it("passes an existing SafeError through unchanged so its kind is not masked", () => {
|
||||
const timeout = new SafeError("Python script timed out", {
|
||||
kind: "operational",
|
||||
code: "timeout",
|
||||
});
|
||||
expect(toSidecarError(timeout, "Upscaling failed")).toBe(timeout);
|
||||
});
|
||||
|
||||
it("uses an Error reason's message", () => {
|
||||
const err = toSidecarError(new Error("model weights corrupt"), "Upscaling failed");
|
||||
expect(isSafeMessageError(err)).toBe(true);
|
||||
expect(err.message).toBe("model weights corrupt");
|
||||
});
|
||||
});
|
||||
|
||||
describe("wrapper propagation (sidecar reason survives the Sentry scrubber)", () => {
|
||||
let png: Buffer;
|
||||
beforeAll(async () => {
|
||||
png = await sharp({
|
||||
create: { width: 4, height: 4, channels: 4, background: { r: 0, g: 0, b: 0, alpha: 0 } },
|
||||
})
|
||||
.png()
|
||||
.toBuffer();
|
||||
});
|
||||
|
||||
it("upscale throws a SafeError carrying the sidecar reason", async () => {
|
||||
runPythonWithProgress.mockResolvedValue({
|
||||
stdout: JSON.stringify({ success: false, error: "RealESRGAN weights not found" }),
|
||||
});
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
await upscale(png, tmpdir(), { scale: 2 });
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(isSafeMessageError(caught)).toBe(true);
|
||||
expect((caught as SafeError).message).toBe("RealESRGAN weights not found");
|
||||
expect((caught as SafeError).kind).toBe("bug");
|
||||
});
|
||||
|
||||
it("transcribeAudio throws a SafeError carrying the sidecar reason", async () => {
|
||||
runPythonWithProgress.mockResolvedValue({
|
||||
stdout: JSON.stringify({ error: "audio stream unreadable" }),
|
||||
});
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
await transcribeAudio("/nonexistent/input.wav", {});
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(isSafeMessageError(caught)).toBe(true);
|
||||
expect((caught as SafeError).message).toBe("audio stream unreadable");
|
||||
});
|
||||
});
|
||||
@@ -23,7 +23,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
}));
|
||||
|
||||
// Mock the bridge module
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
isGpuAvailable: vi.fn(() => false),
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -15,7 +15,8 @@ vi.mock("node:fs/promises", () => ({
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../../../packages/ai/src/bridge.js", () => ({
|
||||
vi.mock("../../../packages/ai/src/bridge.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../../packages/ai/src/bridge.js")>()),
|
||||
runPythonWithProgress: vi.fn(),
|
||||
parseStdoutJson: vi.fn(),
|
||||
isGpuAvailable: vi.fn().mockReturnValue(false),
|
||||
|
||||
Reference in New Issue
Block a user