diff --git a/src/app/api/proxy/route.ts b/src/app/api/proxy/route.ts index 1e30742..6aacc35 100644 --- a/src/app/api/proxy/route.ts +++ b/src/app/api/proxy/route.ts @@ -342,10 +342,10 @@ export async function POST(request: NextRequest) { if (!body.messages || body.messages.length < 2) { if (isStreamingRequest) { - return createSSEErrorResponse(TOO_FEW_MESSAGES.message); + return createSSEErrorResponse(userFacingMessage(TOO_FEW_MESSAGES)); } return NextResponse.json( - { error: TOO_FEW_MESSAGES.message }, + { error: userFacingMessage(TOO_FEW_MESSAGES) }, { status: 400, headers: { diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 98ce706..25f2fe1 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -3,19 +3,26 @@ * * Every failure used to produce one message: "You dingus, read the directions * on sucker before trying again", with a note about context size. For a - * truncated card that was fair. For a request shape the user never chose, or - * for a bug in sucker, it blamed them for something they could not have done - * anything about — and told them to check directions they had already followed. + * truncated card that was fair. For anything else it blamed the user for + * something they could not have done anything about. * - * So: anything thrown as a SuckerError is written for the person who hit it - * and is shown to them verbatim. Anything else is a bug nobody anticipated, - * and the reply says so and asks them to report it, because an unexplained - * failure that nobody hears about never gets fixed. + * The split that matters is not explained-vs-unexplained, it is + * **fixable-vs-not**: + * + * - A truncated card is the one failure a user can act on: raise the + * context size. That message is shown verbatim. + * - Everything else — a request with no card in it, no greeting, nothing + * at all — cannot happen through Janitor, and if it does the user has no + * way to cause or fix it. Telling them to "open the chat properly" would + * be the old message wearing a new coat. Those say plainly that it is not + * their fault and ask them to report it, carrying a short tag so the + * report identifies which assumption broke. */ /** Where to send someone when sucker is at fault. */ export const CONTACT = "DM @lyseverian on Discord"; +/** A failure the person who hit it can actually do something about. */ export class SuckerError extends Error { constructor(message: string) { super(message); @@ -24,29 +31,52 @@ export class SuckerError extends Error { } /** - * `instanceof` is checked by name as well: Next can bundle the same module - * into more than one chunk, and a class identity that differs between them - * would quietly send every explained error down the "unknown bug" path. + * A request shaped in a way Janitor does not produce. Not the user's fault + * and not theirs to fix; `detail` exists so their report says which + * assumption broke, not so they can act on it. */ +export class ImpossibleRequest extends Error { + readonly detail: string; + constructor(detail: string) { + super(`impossible request: ${detail}`); + this.name = "ImpossibleRequest"; + this.detail = detail; + } +} + +/** + * Both are checked by name as well as by identity: Next can bundle a module + * into more than one chunk, and a mismatched class would quietly send every + * classified error down the wrong path. + */ +const named = (error: unknown, name: string): boolean => + typeof error === "object" && + error !== null && + (error as { name?: unknown }).name === name; + export function isSuckerError(error: unknown): error is SuckerError { - return ( - error instanceof SuckerError || - (typeof error === "object" && - error !== null && - (error as { name?: unknown }).name === "SuckerError") - ); + return error instanceof SuckerError || named(error, "SuckerError"); +} + +export function isImpossibleRequest(error: unknown): error is ImpossibleRequest { + return error instanceof ImpossibleRequest || named(error, "ImpossibleRequest"); } export function userFacingMessage(error: unknown): string { if (isSuckerError(error)) return error.message; + const detail = isImpossibleRequest(error) ? ` (${error.detail})` : ""; return ( "Something went wrong that sucker has no explanation for, and nothing was " + - `saved. This one is not your fault — please ${CONTACT} and say what you ` + - "were pulling, because unreported ones do not get fixed." + `saved. This one is not your fault and there is nothing for you to fix — ` + + `please ${CONTACT} and say what you were pulling, because unreported ones ` + + `do not get fixed.${detail}` ); } -/** The card arrived without the markers sucker needs to read it. */ +/** + * The one failure a user can act on: Janitor cut the card short because the + * bot is bigger than their context size, so the markers sucker reads are gone. + */ export const TRUNCATED_CARD = new SuckerError( "The card came through incomplete — the markers sucker looks for are not in " + "it. That usually means the bot is larger than your context size and " + @@ -54,18 +84,9 @@ export const TRUNCATED_CARD = new SuckerError( `try again. If it is already generous, re-read the directions on sucker, and ${CONTACT} if it still will not go.` ); -export const NO_CARD = new SuckerError( - "There is no character card in that request — sucker could not find a " + - "system message in it. Point sucker at a chat with a bot in it, not an " + - "empty one, and try again." -); - -export const NO_GREETING = new SuckerError( - "That chat has no message from the bot yet, so there is nothing to pull. " + - "Open the chat until the bot's first message shows, then send again." -); - -export const TOO_FEW_MESSAGES = new SuckerError( - "That request barely had anything in it — sucker needs a chat with a bot " + - "and at least one message in it. Open the chat properly and try again." -); +// None of these can happen through Janitor. They exist so that if one ever +// does, the reply is honest about whose problem it is and the report says +// which assumption broke. +export const NO_CARD = new ImpossibleRequest("no system message in the request"); +export const NO_GREETING = new ImpossibleRequest("no greeting in the request"); +export const TOO_FEW_MESSAGES = new ImpossibleRequest("fewer than two messages in the request"); diff --git a/tests/errors.test.ts b/tests/errors.test.ts index e596eb1..2b4983b 100644 --- a/tests/errors.test.ts +++ b/tests/errors.test.ts @@ -1,44 +1,63 @@ /** - * Which failures explain themselves, and which ask to be reported. + * Which failures a user can act on, and which just need reporting. */ import { test } from "node:test"; import assert from "node:assert/strict"; import { CONTACT, + ImpossibleRequest, NO_CARD, NO_GREETING, SuckerError, TOO_FEW_MESSAGES, TRUNCATED_CARD, + isImpossibleRequest, isSuckerError, userFacingMessage, } from "../src/lib/errors.ts"; -test("an explained failure is shown to the user as written", () => { - for (const e of [NO_CARD, NO_GREETING, TOO_FEW_MESSAGES, TRUNCATED_CARD]) { - assert.equal(userFacingMessage(e), e.message); - assert.ok(!/dingus/i.test(e.message), "nobody is called a dingus for these"); - } -}); - -test("an unexplained failure owns it and asks to be reported", () => { - const msg = userFacingMessage(new TypeError("Cannot read properties of undefined")); - assert.match(msg, /not your fault/i); - assert.ok(msg.includes(CONTACT), "must say where to report it"); - // The internals must not leak into a user-facing reply. - assert.ok(!msg.includes("Cannot read properties")); -}); - -test("the truncated-card case keeps the context-size advice", () => { +test("the one fixable failure is shown as written, with the advice that fixes it", () => { + assert.equal(userFacingMessage(TRUNCATED_CARD), TRUNCATED_CARD.message); assert.match(TRUNCATED_CARD.message, /Context Size/); assert.ok(TRUNCATED_CARD.message.includes(CONTACT)); }); -test("recognised across module boundaries, not just by identity", () => { - assert.ok(isSuckerError(new SuckerError("x"))); - // What a second copy of the class in another bundle would look like. - assert.ok(isSuckerError({ name: "SuckerError", message: "x" })); - assert.ok(!isSuckerError(new Error("x"))); - assert.ok(!isSuckerError(null)); - assert.ok(!isSuckerError("SuckerError")); +test("shapes Janitor cannot produce never tell the user to fix them", () => { + for (const e of [NO_CARD, NO_GREETING, TOO_FEW_MESSAGES]) { + const msg = userFacingMessage(e); + assert.match(msg, /not your fault and there is nothing for you to fix/i); + assert.ok(msg.includes(CONTACT), "must say where to report it"); + // The old failure mode: instructions for something they cannot do. + assert.ok(!/open the chat|try again|point sucker/i.test(msg), `actionable advice leaked into: ${msg}`); + } +}); + +test("the report carries which assumption broke, so it is useful", () => { + assert.match(userFacingMessage(NO_GREETING), /\(no greeting in the request\)/); + assert.match(userFacingMessage(NO_CARD), /\(no system message in the request\)/); + assert.match(userFacingMessage(TOO_FEW_MESSAGES), /\(fewer than two messages/); +}); + +test("a genuine bug reports itself without leaking internals", () => { + const msg = userFacingMessage(new TypeError("Cannot read properties of undefined")); + assert.match(msg, /not your fault/i); + assert.ok(msg.includes(CONTACT)); + assert.ok(!msg.includes("Cannot read properties")); + assert.ok(!/\($/.test(msg.trim()), "no empty detail parenthesis"); +}); + +test("nobody is called a dingus", () => { + for (const e of [TRUNCATED_CARD, NO_CARD, NO_GREETING, TOO_FEW_MESSAGES, new Error("x")]) { + assert.ok(!/dingus/i.test(userFacingMessage(e))); + } +}); + +test("classified across module boundaries, not just by identity", () => { + assert.ok(isSuckerError(new SuckerError("x"))); + assert.ok(isSuckerError({ name: "SuckerError", message: "x" })); + assert.ok(isImpossibleRequest(new ImpossibleRequest("x"))); + assert.ok(isImpossibleRequest({ name: "ImpossibleRequest", detail: "x" })); + assert.ok(!isSuckerError(new Error("x"))); + assert.ok(!isImpossibleRequest(new SuckerError("x"))); + assert.ok(!isSuckerError(null)); });