chore(deps): bump ibex-client to ^3.3.0 for structured error details

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",

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)

src/services/ibex/errors.ts

@@ -87,22 +87,26 @@ export const errorHandler = <T>(
 ): 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 (<code>): <detail>" 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,29 +115,50 @@ export const errorHandler = <T>(
 }
 
 /**
- * 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))
   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 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: 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)
 }

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,
@@ -56,6 +56,47 @@ 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)
+    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)
+  })
+})
+
 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
@@ -107,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))
 
@@ -191,6 +232,43 @@ 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 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 = `<html>cf-502 error page</html>${"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", () => {

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"