diff --git a/src/services/ibex/client.ts b/src/services/ibex/client.ts index 976c06d99..dcd931cb9 100644 --- a/src/services/ibex/client.ts +++ b/src/services/ibex/client.ts @@ -50,13 +50,7 @@ import { UsdWalletAmount, } from "./types" -import { - errorHandler, - httpErrorHandler, - IbexError, - ParseError, - UnexpectedIbexResponse, -} from "./errors" +import { errorHandler, IbexError, ParseError, UnexpectedIbexResponse } from "./errors" import { ibexWebhookEndpoints, ibexWebhookSecret } from "./webhook-config" const Ibex = new IbexClient( @@ -247,20 +241,11 @@ const payInvoice = async ( webhookSecret: ibexWebhookSecret, } as PayInvoiceV2BodyParam addAttributesToCurrentSpan({ "request.params": JSON.stringify(bodyWithHooks) }) - // Call the generated SDK through withAuth directly (instead of - // Ibex.payInvoiceV2) so a failed payment's FetchError — which carries the - // parsed IBEX error body on `.data` — reaches httpErrorHandler intact. - // This seam predates ibex-client@3.3.0: 3.2.0's ApiError wrapper discarded - // that body (lnflash/ibex-client#6), which made "insufficient balance" 400s - // unclassifiable. As of 3.3.0, ApiError carries the body itself and - // errorHandler classifies it through the standard path, so the seam is - // redundant — retained only as belt-and-braces until it is collapsed back - // to `Ibex.payInvoiceV2(bodyWithHooks).then(errorHandler)` in its own PR - // (lnflash/flash#478; a payment-path change doesn't belong in a deps bump). - return Ibex.authentication - .withAuth(() => Ibex.ibex.payInvoiceV2(bodyWithHooks)) - .then(errorHandler) - .catch(httpErrorHandler) + // Standard SDK path. A raw-fetch seam lived here while ibex-client 3.2.0's + // ApiError discarded the response body (lnflash/ibex-client#6); 3.3.0's + // ApiError carries the body itself (`ibexMessage`/`ibexResponse`/`httpCode`) + // and errorHandler classifies it, so the seam was collapsed (lnflash/flash#478). + return Ibex.payInvoiceV2(bodyWithHooks).then(errorHandler) } // onchain transactions are typically high-value diff --git a/src/services/ibex/errors.ts b/src/services/ibex/errors.ts index f3d09bbe7..8f9f8ef0b 100644 --- a/src/services/ibex/errors.ts +++ b/src/services/ibex/errors.ts @@ -53,7 +53,10 @@ export class CompletedInvoice extends IbexError {} * - ibex-client > 3.2.0 (lnflash/ibex-client#6): ApiError carries the parsed * body on `ibexResponse` and the extracted text on `ibexMessage` * - the generated api SDK's FetchError: parsed JSON body on `.data`, - * typically `{ "error": "..." }`, sometimes `{ "message": "..." }` or text + * typically `{ "error": "..." }`, sometimes `{ "message": "..." }` or text. + * No live caller passes this shape anymore — the payInvoice raw-fetch seam + * was collapsed onto the SDK path (lnflash/flash#478) — but the fallback is + * a few lines and keeps this helper safe for any future raw caller. */ export const ibexErrorDetail = (e: unknown): string | undefined => { if (typeof e !== "object" || e === null) return undefined @@ -71,8 +74,8 @@ export const ibexErrorDetail = (e: unknown): string | undefined => { // The single needle list mapping IBEX error prose to a typed error class. // Matching is case-insensitive (the haystack is lowercased once here) so a // vendor rewording like "Insufficient Balance" cannot silently revert -// classification to the generic path. Both errorHandler and httpErrorHandler -// classify through this helper — never add a needle anywhere else. +// classification to the generic path. errorHandler classifies through this +// helper — never add a needle anywhere else. const classifyIbexErrorText = ( text: string, ): typeof InsufficientIbexBalance | typeof CompletedInvoice | undefined => { @@ -89,9 +92,9 @@ export const errorHandler = ( if (e instanceof ApiError) { // Classify against the structured body detail when the error carries one // (ibex-client >= 3.3.0's ApiError extracts it onto `ibexMessage`), and - // against `message` otherwise — flash's raw-fetch path embeds the body - // text in the message. Body-carrying shapes are checked first so a stack - // that happens to contain a needle can't misclassify. + // against `message` otherwise — a defensive fallback for shapes that only + // embed the body text in the message. Body-carrying shapes are checked + // first so a stack that happens to contain a needle can't misclassify. const detail = ibexErrorDetail(e) const classified = classifyIbexErrorText(detail ?? e.message) if (classified === InsufficientIbexBalance) @@ -113,52 +116,3 @@ export const errorHandler = ( if (e instanceof IbexClientError) return new IbexError(e, ErrorLevel.Warn) return e } - -/** - * Classify a raw error thrown by the generated IBEX SDK (or fetch) for call - * sites that invoke the SDK through `Ibex.authentication.withAuth` themselves - * and route the caught error here (the payInvoice raw-fetch seam). The seam - * predates ibex-client@3.3.0: 3.2.0's ApiError kept only `httpCode` and - * discarded the JSON error body that distinguishes e.g. "insufficient - * balance" from any other 400 (lnflash/ibex-client#6). As of 3.3.0, ApiError - * extracts the body itself (`ibexResponse` / `ibexMessage`) and errorHandler - * classifies it through the standard path — this handler remains as - * defense-in-depth for the raw-fetch seam until that seam is collapsed - * (lnflash/flash#478). - */ -export const httpErrorHandler = (e: unknown): IbexError => { - const raw = e instanceof Error ? e : new Error(String(e)) - if (raw instanceof AuthenticationError) return new IbexError(raw, ErrorLevel.Critical) - // ApiError's constructor keeps `.status` as httpCode, which IbexError reads. - const wrapped = raw instanceof IbexClientError ? raw : new ApiError(raw) - // Derive the detail from `wrapped`, never from `raw`. For a raw FetchError, - // ApiError's own extraction (`ibexMessage`) is capped at ibex-client's - // MAX_IBEX_MESSAGE_LENGTH, while reading straight off `raw.data` is not: - // comparing an uncapped detail against the capped copy embedded in - // `wrapped.message` would defeat the dedupe guard below for any body over - // the cap — exactly the Cloudflare-HTML-error-page outage the cap exists - // for — and prepend the full multi-KB body onto every failing call's - // message. When `raw` is already an IbexClientError, wrapped === raw and - // the extraction is unchanged. The uncapped body exists only on the local - // ApiError's `ibexResponse` and does not survive onto the returned - // IbexError — by design: pino serializes own enumerable properties, and - // attaching a multi-KB body would recreate the log bloat the cap prevents. - const detail = ibexErrorDetail(wrapped) - const classified = classifyIbexErrorText(detail ?? raw.message) - if (classified === InsufficientIbexBalance) - return new InsufficientIbexBalance(wrapped, ErrorLevel.Info, detail) - if (classified === CompletedInvoice) - return new CompletedInvoice(wrapped, ErrorLevel.Info) - // Unclassified path: an unrecognized IBEX 400 must still log what IBEX - // actually said, not just "FetchError: Bad Request" + stack. The ApiError - // constructed above already embeds the extracted detail in its message - // (ibex-client >= 3.3.0), so only carry the detail when the message doesn't - // already contain it — never append the same text twice. - if ( - detail !== undefined && - !(raw instanceof IbexClientError) && - !wrapped.message.includes(detail) - ) - wrapped.message = `${detail}\n${wrapped.message}` - return new IbexError(wrapped, ErrorLevel.Warn) -} diff --git a/test/flash/unit/services/ibex/errors.spec.ts b/test/flash/unit/services/ibex/errors.spec.ts index de480b7b8..ace26fe4e 100644 --- a/test/flash/unit/services/ibex/errors.spec.ts +++ b/test/flash/unit/services/ibex/errors.spec.ts @@ -4,7 +4,6 @@ import { ApiError, AuthenticationError, MAX_IBEX_MESSAGE_LENGTH } from "ibex-cli import { CompletedInvoice, errorHandler, - httpErrorHandler, IbexError, ibexErrorDetail, InsufficientIbexBalance, @@ -58,7 +57,10 @@ describe("ibexErrorDetail", () => { // ibex-client >= 3.3.0 populates ibexMessage/ibexResponse/httpCode itself — // these construct the real ApiError with no simulated fields, pinning the -// integration the ^3.3.0 bump claims to deliver. +// integration the ^3.3.0 bump claims to deliver. Since the payInvoice +// raw-fetch seam was collapsed (lnflash/flash#478), this is THE error shape +// every failed SDK call resolves with: ibex-client's wrapper catches whatever +// the generated SDK throws and hands errorHandler `new ApiError(thrown)`. describe("ibex-client 3.3.0 integration", () => { it("ApiError extracts the body detail on construction", () => { const apiErr = new ApiError(fetchErrorShaped(400, { error: insufficientDetail })) @@ -76,11 +78,48 @@ describe("ibex-client 3.3.0 integration", () => { expect(result).toBeInstanceOf(InsufficientIbexBalance) const err = result as InsufficientIbexBalance + expect(err.httpCode).toBe(400) + expect(err.level).toBe(ErrorLevel.Info) expect(err.message).toBe(insufficientDetailStripped) expect(err.message).not.toContain("39c6e986-979b-40ab-9e7b-df18a9277a84") expect(err.detail).toBe(insufficientDetail) }) + it("classifies a 400 with a non-JSON insufficient-balance body", () => { + const apiErr = new ApiError(fetchErrorShaped(400, "insufficient balance (text body)")) + + const result = errorHandler(apiErr) + + expect(result).toBeInstanceOf(InsufficientIbexBalance) + expect((result as InsufficientIbexBalance).message).toBe( + "insufficient balance (text body)", + ) + }) + + it("classifies case-insensitively, preserving IBEX's original casing in the detail", () => { + const apiErr = new ApiError( + fetchErrorShaped(400, { error: "Insufficient Balance. Current Balance: 5.000000" }), + ) + + const result = errorHandler(apiErr) + + expect(result).toBeInstanceOf(InsufficientIbexBalance) + expect((result as InsufficientIbexBalance).message).toBe( + "Insufficient Balance. Current Balance: 5.000000", + ) + }) + + it("classifies payment-already-prepared from a real body-carrying ApiError", () => { + const apiErr = new ApiError( + fetchErrorShaped(400, { error: "Payment Already Prepared" }), + ) + + const result = errorHandler(apiErr) + + expect(result).toBeInstanceOf(CompletedInvoice) + expect((result as CompletedInvoice).level).toBe(ErrorLevel.Info) + }) + it("errorHandler carries a real unclassified detail onto the generic IbexError", () => { const apiErr = new ApiError(fetchErrorShaped(400, { error: "invalid parameters" })) @@ -88,12 +127,70 @@ describe("ibex-client 3.3.0 integration", () => { expect(result).toBeInstanceOf(IbexError) expect(result).not.toBeInstanceOf(InsufficientIbexBalance) - const message = (result as IbexError).message - expect(message).toContain("invalid parameters") + const err = result as IbexError + // an unrecognized IBEX 400 that logs only "FetchError: Bad Request" is the + // debugging blindness this module exists to fix + expect(err.httpCode).toBe(400) + expect(err.level).toBe(ErrorLevel.Warn) + expect(err.message).toContain("invalid parameters") // 3.3.0's ApiError already embeds the detail in its own message — the // unclassified carry must not append the same text a second time (the // duplicated detail would reach logs and Discord alert embeds) - expect(message.split("invalid parameters").length - 1).toBe(1) + expect(err.message.split("invalid parameters").length - 1).toBe(1) + }) + + it("keeps the message bounded when the body exceeds ibex-client's cap", () => { + // Cloudflare-style outage: IBEX's proxy returns a full HTML error page, + // which arrives as a plain-text body far over MAX_IBEX_MESSAGE_LENGTH. + // ApiError embeds only the capped copy (`ibexMessage`) in its message and + // errorHandler derives the detail from that same capped extraction, so + // the dedupe guard matches and nothing multi-KB is ever prepended into + // logs or Discord alert embeds. The full body survives only on the local + // ApiError's `ibexResponse`, which does not reach the returned IbexError. + const hugeBody = `cf-502 error page${"x".repeat( + MAX_IBEX_MESSAGE_LENGTH * 4, + )}` + const raw = fetchErrorShaped(502, hugeBody) + const rawStackLength = (raw.stack as string).length + + const result = errorHandler(new ApiError(raw)) + + expect(result).toBeInstanceOf(IbexError) + expect(result).not.toBeInstanceOf(InsufficientIbexBalance) + const err = result as IbexError + expect(err.httpCode).toBe(502) + // the truncated detail appears exactly once... + const truncatedDetail = `${hugeBody.slice(0, MAX_IBEX_MESSAGE_LENGTH)}... [truncated]` + expect(err.message.split(truncatedDetail).length - 1).toBe(1) + // ... the capped prefix itself is not duplicated (truncated + full copy)... + expect(err.message.split(hugeBody.slice(0, MAX_IBEX_MESSAGE_LENGTH)).length - 1).toBe( + 1, + ) + // ... the full uncapped body never reaches the message... + expect(err.message).not.toContain(hugeBody) + // ... and the whole message stays bounded: stack + capped detail + framing + expect(err.message.length).toBeLessThan( + rawStackLength + MAX_IBEX_MESSAGE_LENGTH + 100, + ) + }) + + it("maps an ApiError wrapping a network-level failure to a generic IbexError without a status", () => { + // the wrapper catches TypeError("fetch failed") like anything else thrown + const result = errorHandler(new ApiError(new TypeError("fetch failed"))) + + expect(result).toBeInstanceOf(IbexError) + expect(result).not.toBeInstanceOf(InsufficientIbexBalance) + const err = result as IbexError + expect(err.httpCode).toBeUndefined() + expect(err.level).toBe(ErrorLevel.Warn) + }) + + it("tolerates an ApiError wrapping a non-Error throwable", () => { + // `throw "boom"` somewhere under the SDK still resolves to ApiError("boom") + const result = errorHandler(new ApiError("boom" as unknown as Error)) + + expect(result).toBeInstanceOf(IbexError) + expect((result as IbexError).httpCode).toBeUndefined() }) }) @@ -157,6 +254,8 @@ describe("errorHandler", () => { expect(result).toBeInstanceOf(IbexError) expect(result).not.toBeInstanceOf(InsufficientIbexBalance) + // the status survives even when the response carried no body + expect((result as IbexError).httpCode).toBe(400) }) it("classifies payment-already-prepared as CompletedInvoice", () => { @@ -189,138 +288,3 @@ describe("errorHandler", () => { expect(errorHandler(response)).toBe(response) }) }) - -describe("httpErrorHandler", () => { - it("classifies a 400 with a JSON insufficient-balance body", () => { - const raw = fetchErrorShaped(400, { error: insufficientDetail }) - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(InsufficientIbexBalance) - const err = result as InsufficientIbexBalance - expect(err.httpCode).toBe(400) - expect(err.level).toBe(ErrorLevel.Info) - // client-facing message keeps the IBEX detail minus the internal account UUID - expect(err.message).toBe(insufficientDetailStripped) - expect(err.message).not.toContain("account:") - // the unstripped vendor text is preserved for logs/spans - expect(err.detail).toBe(insufficientDetail) - }) - - it("classifies a 400 with a non-JSON insufficient-balance body", () => { - const raw = fetchErrorShaped(400, "insufficient balance (text body)") - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(InsufficientIbexBalance) - expect((result as InsufficientIbexBalance).message).toBe( - "insufficient balance (text body)", - ) - }) - - it("maps other 400s to a generic IbexError, keeping the status", () => { - const raw = fetchErrorShaped(400, { error: "invalid parameters" }) - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(IbexError) - expect(result).not.toBeInstanceOf(InsufficientIbexBalance) - const err = result as IbexError - expect(err.httpCode).toBe(400) - expect(err.level).toBe(ErrorLevel.Warn) - // the extracted body detail must survive the generic path — an - // unrecognized IBEX 400 that logs only "FetchError: Bad Request" is the - // debugging blindness this module exists to fix - expect(err.message).toContain("invalid parameters") - // ... and exactly once: 3.3.0's ApiError wrapper already embeds the - // detail in its message, so the carry must not duplicate it - expect(err.message.split("invalid parameters").length - 1).toBe(1) - }) - - it("keeps the message bounded when the body exceeds ibex-client's cap", () => { - // Cloudflare-style outage: IBEX's proxy returns a full HTML error page, - // which arrives as a plain-text body far over MAX_IBEX_MESSAGE_LENGTH. - // ApiError embeds only the capped copy in its message; if the dedupe - // guard compared against an uncapped extraction of the same body, it - // would always miss and prepend the full multi-KB blob — once per - // failing call — into logs and Discord alert embeds. - const hugeBody = `cf-502 error page${"x".repeat( - MAX_IBEX_MESSAGE_LENGTH * 4, - )}` - const raw = fetchErrorShaped(502, hugeBody) - const rawStackLength = (raw.stack as string).length - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(IbexError) - expect(result).not.toBeInstanceOf(InsufficientIbexBalance) - const err = result as IbexError - expect(err.httpCode).toBe(502) - // the truncated detail appears exactly once... - const truncatedDetail = `${hugeBody.slice(0, MAX_IBEX_MESSAGE_LENGTH)}... [truncated]` - expect(err.message.split(truncatedDetail).length - 1).toBe(1) - // ... the capped prefix itself is not duplicated (truncated + full copy)... - expect(err.message.split(hugeBody.slice(0, MAX_IBEX_MESSAGE_LENGTH)).length - 1).toBe( - 1, - ) - // ... the full uncapped body never reaches the message... - expect(err.message).not.toContain(hugeBody) - // ... and the whole message stays bounded: stack + capped detail + framing - expect(err.message.length).toBeLessThan( - rawStackLength + MAX_IBEX_MESSAGE_LENGTH + 100, - ) - }) - - it("keeps the generic path unchanged when the error carries no body detail", () => { - const raw = fetchErrorShaped(500, undefined) - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(IbexError) - expect((result as IbexError).httpCode).toBe(500) - }) - - it("classifies insufficient-balance case-insensitively", () => { - const raw = fetchErrorShaped(400, { - error: "Insufficient Balance. Current Balance: 5.000000", - }) - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(InsufficientIbexBalance) - // the client-facing detail keeps IBEX's original casing - expect((result as InsufficientIbexBalance).message).toBe( - "Insufficient Balance. Current Balance: 5.000000", - ) - }) - - it("classifies payment-already-prepared case-insensitively", () => { - const raw = fetchErrorShaped(400, { error: "Payment Already Prepared" }) - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(CompletedInvoice) - }) - - it("classifies payment-already-prepared as CompletedInvoice", () => { - const raw = fetchErrorShaped(400, { error: "payment already prepared" }) - - const result = httpErrorHandler(raw) - - expect(result).toBeInstanceOf(CompletedInvoice) - }) - - it("maps network-level failures to a generic IbexError without a status", () => { - const result = httpErrorHandler(new TypeError("fetch failed")) - - expect(result).toBeInstanceOf(IbexError) - expect(result).not.toBeInstanceOf(InsufficientIbexBalance) - expect((result as IbexError).httpCode).toBeUndefined() - }) - - it("tolerates non-Error throwables", () => { - const result = httpErrorHandler("boom") - - expect(result).toBeInstanceOf(IbexError) - }) -}) diff --git a/test/flash/unit/services/ibex/pay-invoice.spec.ts b/test/flash/unit/services/ibex/pay-invoice.spec.ts index 3c4bb2530..41f83e72d 100644 --- a/test/flash/unit/services/ibex/pay-invoice.spec.ts +++ b/test/flash/unit/services/ibex/pay-invoice.spec.ts @@ -1,5 +1,4 @@ const mockPayInvoiceV2 = jest.fn() -const mockWithAuth = jest.fn() const mockGetAccessToken = jest.fn() const mockSetAccessToken = jest.fn() @@ -30,26 +29,41 @@ jest.mock("@services/ibex/webhook-server", () => ({ }, })) -// Keep the REAL ibex-client error classes (errorHandler/httpErrorHandler rely -// on instanceof against them); mock only the client instance so we can drive -// the SDK call underneath payInvoice. -jest.mock("ibex-client", () => ({ - ...jest.requireActual("ibex-client"), - __esModule: true, - default: jest.fn().mockImplementation(() => ({ - authentication: { - storage: { - getAccessToken: (...args: unknown[]) => mockGetAccessToken(...args), - setAccessToken: (...args: unknown[]) => mockSetAccessToken(...args), - setRefreshToken: jest.fn(), +// Keep the REAL ibex-client error classes (errorHandler relies on instanceof +// against them); mock only the client instance so we can drive the SDK call +// underneath payInvoice. The mocked payInvoiceV2 mirrors ibex-client v3.3.0's +// wrapper contract, which the collapsed payInvoice path +// (`Ibex.payInvoiceV2(body).then(errorHandler)`, lnflash/flash#478) relies on: +// `withAuth` unwraps `.data` on success or resolves with an +// AuthenticationError, and anything thrown resolves as `new ApiError(thrown)`. +jest.mock("ibex-client", () => { + const actual = jest.requireActual("ibex-client") + return { + ...actual, + __esModule: true, + default: jest.fn().mockImplementation(() => ({ + authentication: { + storage: { + getAccessToken: (...args: unknown[]) => mockGetAccessToken(...args), + setAccessToken: (...args: unknown[]) => mockSetAccessToken(...args), + setRefreshToken: jest.fn(), + }, }, - withAuth: (...args: unknown[]) => mockWithAuth(...args), - }, - ibex: { - payInvoiceV2: (...args: unknown[]) => mockPayInvoiceV2(...args), - }, - })), -})) + payInvoiceV2: async (body: unknown) => { + try { + const resp = await mockPayInvoiceV2(body) + if (resp instanceof actual.AuthenticationError) return resp + return (resp as { data: unknown }).data + } catch (err) { + return new actual.ApiError(err as Error) + } + }, + })), + } +}) + +import { ErrorLevel } from "@domain/shared" +import { AuthenticationError } from "ibex-client" import Ibex from "@services/ibex/client" import { IbexError, InsufficientIbexBalance } from "@services/ibex/errors" @@ -68,10 +82,6 @@ const fetchErrorShaped = (status: number, data: unknown): Error => { describe("Ibex.payInvoice error classification", () => { beforeEach(() => { jest.clearAllMocks() - // mirror the real withAuth: run the SDK call, unwrap .data, rethrow errors - mockWithAuth.mockImplementation( - async (apiCall: () => Promise<{ data: unknown }>) => (await apiCall()).data, - ) }) const payArgs = { @@ -120,6 +130,17 @@ describe("Ibex.payInvoice error classification", () => { expect((result as IbexError).httpCode).toBeUndefined() }) + it("returns a critical IbexError when authentication fails", async () => { + // withAuth resolves with (never throws) an AuthenticationError + mockPayInvoiceV2.mockResolvedValue(new AuthenticationError("auth failed")) + + const result = await Ibex.payInvoice(payArgs) + + expect(result).toBeInstanceOf(IbexError) + expect(result).not.toBeInstanceOf(InsufficientIbexBalance) + expect((result as IbexError).level).toBe(ErrorLevel.Critical) + }) + it("passes successful payments through with the webhook body", async () => { const response = { transaction: { payment: { status: { id: 2 } } },