From 35b8fd294bf5788d02363ab4d0f8512dec732533 Mon Sep 17 00:00:00 2001 From: Dread Date: Wed, 12 Aug 2026 08:06:40 -0700 Subject: [PATCH 1/4] chore(deps): bump ibex-client to ^3.3.0 for structured error details ibex-client 3.3.0 (lnflash/ibex-client#12) populates httpCode, ibexResponse, and ibexMessage on ApiError at construction, so every IBEX call site now gets the response detail #476 could only extract at the payInvoice raw-fetch seam under pinned 3.2.0. No code changes needed: errors.ts already duck-types ibexMessage. New integration tests construct the real 3.3.0 ApiError (no simulated fields) and pin extraction, insufficient-balance classification with account-id stripping, and the unclassified fall-through detail. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015fAxSGEsL2LsMx1uCjAzHH --- package.json | 2 +- test/flash/unit/services/ibex/errors.spec.ts | 36 ++++++++++++++++++++ yarn.lock | 8 ++--- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index a3832d2ae..23ffb2123 100644 --- a/package.json +++ b/package.json @@ -97,7 +97,7 @@ "graphql-ws": "^5.13.1", "gt3-server-node-express-sdk": "https://github.com/GaloyMoney/gt3-server-node-express-bypass#master", "i18n": "^0.15.1", - "ibex-client": "^3.2.0", + "ibex-client": "^3.3.0", "invoices": "^3.0.0", "ioredis": "^5.3.2", "ioredis-cache": "^2.0.0", diff --git a/test/flash/unit/services/ibex/errors.spec.ts b/test/flash/unit/services/ibex/errors.spec.ts index fa9283bb4..a2911053d 100644 --- a/test/flash/unit/services/ibex/errors.spec.ts +++ b/test/flash/unit/services/ibex/errors.spec.ts @@ -56,6 +56,42 @@ 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. +describe("ibex-client 3.3.0 integration", () => { + it("ApiError extracts the body detail on construction", () => { + const apiErr = new ApiError(fetchErrorShaped(400, { error: insufficientDetail })) + + expect(apiErr.httpCode).toBe(400) + expect(apiErr.ibexMessage).toBe(insufficientDetail) + expect(apiErr.ibexResponse).toEqual({ error: insufficientDetail }) + expect(ibexErrorDetail(apiErr)).toBe(insufficientDetail) + }) + + it("errorHandler classifies a real body-carrying ApiError, stripping the account id", () => { + const apiErr = new ApiError(fetchErrorShaped(400, { error: insufficientDetail })) + + const result = errorHandler(apiErr) + + expect(result).toBeInstanceOf(InsufficientIbexBalance) + const err = result as InsufficientIbexBalance + expect(err.message).toBe(insufficientDetailStripped) + expect(err.message).not.toContain("39c6e986-979b-40ab-9e7b-df18a9277a84") + expect(err.detail).toBe(insufficientDetail) + }) + + it("errorHandler carries a real unclassified detail onto the generic IbexError", () => { + const apiErr = new ApiError(fetchErrorShaped(400, { error: "invalid parameters" })) + + const result = errorHandler(apiErr) + + expect(result).toBeInstanceOf(IbexError) + expect(result).not.toBeInstanceOf(InsufficientIbexBalance) + expect((result as IbexError).message).toContain("invalid parameters") + }) +}) + describe("errorHandler", () => { it("classifies an ApiError whose message carries the insufficient-balance text", () => { // flash's raw-fetch path embeds the body text in the wrapped message diff --git a/yarn.lock b/yarn.lock index 4d189dd80..c4a302261 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8878,10 +8878,10 @@ i18n@^0.15.1: math-interval-parser "^2.0.1" mustache "^4.2.0" -ibex-client@^3.2.0: - version "3.2.0" - resolved "https://registry.yarnpkg.com/ibex-client/-/ibex-client-3.2.0.tgz#2a9a707e1c250c1bd226e7be99c5d5baa2f24ca2" - integrity sha512-jteyXwSBUIwEyjEXFXdJMUYtBdoZ2HloDoFuBm20rThN6jlR/2XAaimlitZjmImKE1u1wCLrAcaAXWH8AcOStA== +ibex-client@^3.3.0: + version "3.3.0" + resolved "https://registry.yarnpkg.com/ibex-client/-/ibex-client-3.3.0.tgz#92b7098a326d830dfb975b281b08e5113fb054f1" + integrity sha512-gMP2Bm5nyMaaQkRAcj7IoIVyujUnc0DPgIOZaOG/QLtaPb2zCzc1Hu4K/ey6boG2cRODq1VipSeUYgwB870LwQ== dependencies: api "^6.1.2" node-cache "^5.1.2" From a8df2bbca6cff7fd497755a81223e8c11f243ab7 Mon Sep 17 00:00:00 2001 From: Dread Date: Wed, 12 Aug 2026 08:19:14 -0700 Subject: [PATCH 2/4] fix(ibex): dedupe error detail carry, refresh stale 3.2.0 comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes for #477: - errorHandler/httpErrorHandler no longer prepend the IBEX body detail when the message already contains it — ibex-client@3.3.0's ApiError embeds "IBEX response (): " itself, so the unclassified carries were logging every unrecognized IBEX error's detail twice (and the duplicate reached Discord alert embeds via #475). Tests extended to pin the detail appearing exactly once (verified failing against the pre-fix code). - Rewrote the payInvoice raw-fetch seam comment: the 3.2.0 body-discarding rationale no longer holds on 3.3.0; the seam is belt-and-braces only, follow-up to collapse it filed as #478. - Rewrote httpErrorHandler's docblock and errorHandler's classification comment to describe 3.3.0 behavior (ApiError carries the body; httpErrorHandler stays as defense-in-depth for the raw-fetch seam). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015fAxSGEsL2LsMx1uCjAzHH --- src/services/ibex/client.ts | 9 +++- src/services/ibex/errors.ts | 53 ++++++++++++-------- test/flash/unit/services/ibex/errors.spec.ts | 14 ++++-- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/services/ibex/client.ts b/src/services/ibex/client.ts index 90819695c..976c06d99 100644 --- a/src/services/ibex/client.ts +++ b/src/services/ibex/client.ts @@ -250,8 +250,13 @@ const payInvoice = async ( // 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. - // ibex-client@3.2.0's own ApiError wrapper discards that body, which is what - // made "insufficient balance" 400s unclassifiable (lnflash/ibex-client#6). + // 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) diff --git a/src/services/ibex/errors.ts b/src/services/ibex/errors.ts index d6c5c42e2..ad4cc7714 100644 --- a/src/services/ibex/errors.ts +++ b/src/services/ibex/errors.ts @@ -87,22 +87,26 @@ export const errorHandler = ( ): T | IbexError => { if (e instanceof AuthenticationError) return new IbexError(e, ErrorLevel.Critical) if (e instanceof ApiError) { - // Classify against the structured body detail when the error carries one, - // and against `message` otherwise (flash's raw-fetch path embeds the body - // text in the message; ibex-client@3.2.0's ApiError message is only the - // wrapped stack, which is why body-carrying shapes are checked first). + // 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. const detail = ibexErrorDetail(e) const classified = classifyIbexErrorText(detail ?? e.message) if (classified === InsufficientIbexBalance) return new InsufficientIbexBalance(e, ErrorLevel.Info, detail) if (classified === CompletedInvoice) return new CompletedInvoice(e, ErrorLevel.Info) - // Unclassified path: mirror httpErrorHandler's carry — an unrecognized - // IBEX 400 whose ApiError has a stack-only message must still log what - // IBEX actually said. Build a new IbexError rather than mutating `e`, - // which the caller may still hold. + // Unclassified path: an unrecognized IBEX 400 must still log what IBEX + // actually said. ibex-client >= 3.3.0's ApiError already appends + // "IBEX response (): " to its own message, so only carry + // the detail when the message doesn't already contain it (older or raw + // shapes with a stack-only message). Build a new IbexError rather than + // mutating `e`, which the caller may still hold. if (detail !== undefined) { const generic = new IbexError(e, ErrorLevel.Warn) - generic.message = `${detail}\n${generic.message}` + if (!generic.message.includes(detail)) + generic.message = `${detail}\n${generic.message}` return generic } } @@ -111,13 +115,16 @@ export const errorHandler = ( } /** - * Classify a raw error thrown by the generated IBEX SDK (or fetch) before - * ibex-client's ApiError wrapper can discard the response body. With the - * pinned ibex-client@3.2.0, ApiError keeps only `httpCode` — the JSON error - * body that distinguishes e.g. "insufficient balance" from any other 400 only - * exists on the underlying FetchError's `.data` (lnflash/ibex-client#6). - * Call sites that need body-level classification invoke the SDK through - * `Ibex.authentication.withAuth` themselves and route the caught error here. + * 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)) @@ -130,10 +137,16 @@ export const httpErrorHandler = (e: unknown): IbexError => { return new InsufficientIbexBalance(wrapped, ErrorLevel.Info, detail) if (classified === CompletedInvoice) return new CompletedInvoice(wrapped, ErrorLevel.Info) - // Unclassified path: ApiError's message is only the wrapped stack, so carry - // the extracted body detail into it — an unrecognized IBEX 400 must still - // log what IBEX actually said, not just "FetchError: Bad Request" + stack. - if (detail !== undefined && !(raw instanceof IbexClientError)) + // 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 a2911053d..c6ba359c8 100644 --- a/test/flash/unit/services/ibex/errors.spec.ts +++ b/test/flash/unit/services/ibex/errors.spec.ts @@ -88,7 +88,12 @@ describe("ibex-client 3.3.0 integration", () => { expect(result).toBeInstanceOf(IbexError) expect(result).not.toBeInstanceOf(InsufficientIbexBalance) - expect((result as IbexError).message).toContain("invalid parameters") + const message = (result as IbexError).message + expect(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) }) }) @@ -143,8 +148,8 @@ describe("errorHandler", () => { expect(apiErr.message).toBe(originalMessage) }) - it("maps a pinned-version SDK-path ApiError (stack-only message) to a generic IbexError", () => { - // ibex-client@3.2.0 discards the response body: message is only the + it("maps an ApiError with no extractable body (stack-only message) to a generic IbexError", () => { + // when the response carries no body, 3.3.0's ApiError message is only the // wrapped FetchError stack ("FetchError: Bad Request\n at ...") const apiErr = new ApiError(fetchErrorShaped(400, undefined)) @@ -227,6 +232,9 @@ describe("httpErrorHandler", () => { // 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 generic path unchanged when the error carries no body detail", () => { From 14fa78bafbd6ebd4be41937196b5ee859526fd41 Mon Sep 17 00:00:00 2001 From: Dread Date: Wed, 12 Aug 2026 08:30:41 -0700 Subject: [PATCH 3/4] fix(ibex): derive dedupe detail from the capped ApiError extraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit httpErrorHandler extracted the detail from the raw FetchError's .data (uncapped) but compared it against the ApiError-wrapped message, which embeds only ibex-client's MAX_IBEX_MESSAGE_LENGTH-truncated copy. For any body over the cap — the Cloudflare-HTML-error-page outage the cap exists for — the includes() guard always missed, prepending the full multi-KB body onto every failing call's message with the first 500 chars duplicated (truncated + full), defeating the upstream cap in logs and Discord alert embeds. Construct the ApiError first and derive the detail from it: both sides of the dedupe comparison now use the same bounded string. When the raw error is already an IbexClientError, wrapped === raw and the extraction is unchanged. The uncapped body remains available on ibexResponse. Adds a regression test feeding a body 4x over MAX_IBEX_MESSAGE_LENGTH through httpErrorHandler, asserting the message stays bounded and the truncated detail appears exactly once (fails against the previous implementation). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015fAxSGEsL2LsMx1uCjAzHH --- src/services/ibex/errors.ts | 12 ++++++- test/flash/unit/services/ibex/errors.spec.ts | 36 +++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/services/ibex/errors.ts b/src/services/ibex/errors.ts index ad4cc7714..120ab67a6 100644 --- a/src/services/ibex/errors.ts +++ b/src/services/ibex/errors.ts @@ -129,9 +129,19 @@ export const errorHandler = ( 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) - const detail = ibexErrorDetail(raw) // 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 stays on + // `wrapped.ibexResponse` for anyone who needs it verbatim. + const detail = ibexErrorDetail(wrapped) const classified = classifyIbexErrorText(detail ?? raw.message) if (classified === InsufficientIbexBalance) return new InsufficientIbexBalance(wrapped, ErrorLevel.Info, detail) diff --git a/test/flash/unit/services/ibex/errors.spec.ts b/test/flash/unit/services/ibex/errors.spec.ts index c6ba359c8..de480b7b8 100644 --- a/test/flash/unit/services/ibex/errors.spec.ts +++ b/test/flash/unit/services/ibex/errors.spec.ts @@ -1,5 +1,5 @@ import { ErrorLevel } from "@domain/shared" -import { ApiError, AuthenticationError } from "ibex-client" +import { ApiError, AuthenticationError, MAX_IBEX_MESSAGE_LENGTH } from "ibex-client" import { CompletedInvoice, @@ -237,6 +237,40 @@ describe("httpErrorHandler", () => { 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) From 0d5470c43662fe3eb7844cf5dd63946b71f0c198 Mon Sep 17 00:00:00 2001 From: Dread Date: Wed, 12 Aug 2026 08:37:14 -0700 Subject: [PATCH 4/4] =?UTF-8?q?docs(ibex):=20correct=20httpErrorHandler=20?= =?UTF-8?q?comment=20=E2=80=94=20ibexResponse=20does=20not=20survive=20ont?= =?UTF-8?q?o=20the=20returned=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 review fix for PR #477: the comment claimed the uncapped body stays reachable on wrapped.ibexResponse, but wrapped is function-local and IbexError's constructor copies only name/httpCode/message — during a Cloudflare-outage scenario a maintainer would search for ibexResponse and find nothing. State reality: the uncapped body is deliberately dropped so pino never serializes a multi-KB blob. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015fAxSGEsL2LsMx1uCjAzHH --- src/services/ibex/errors.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/services/ibex/errors.ts b/src/services/ibex/errors.ts index 120ab67a6..f3d09bbe7 100644 --- a/src/services/ibex/errors.ts +++ b/src/services/ibex/errors.ts @@ -139,8 +139,10 @@ export const httpErrorHandler = (e: unknown): IbexError => { // 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 stays on - // `wrapped.ibexResponse` for anyone who needs it verbatim. + // 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)