From da5d7c85d0d8051a973fd4e9eed98d3b87b85841 Mon Sep 17 00:00:00 2001 From: skyc1e Date: Wed, 1 Apr 2026 13:56:50 +0200 Subject: [PATCH] fix(core): isolate afterVerify and afterSettle hook errors from payment outcomes Lifecycle hooks (afterVerify, afterSettle) ran inside the main try block in both x402ResourceServer and x402Facilitator. If a hook threw, the error fell into the onVerifyFailure / onSettleFailure catch path, incorrectly treating a successful payment as a failure. Wrap each hook invocation in its own try/catch and log the error so hook side-effects (logging, metrics, notifications) cannot corrupt payment state. Closes #1826 --- .../core/src/facilitator/x402Facilitator.ts | 12 ++++- .../core/src/server/x402ResourceServer.ts | 12 ++++- .../facilitator/x402Facilitator.hooks.test.ts | 47 +++++++++++++++++++ .../unit/server/x402ResourceServer.test.ts | 37 +++++++++++++++ 4 files changed, 104 insertions(+), 4 deletions(-) diff --git a/typescript/packages/core/src/facilitator/x402Facilitator.ts b/typescript/packages/core/src/facilitator/x402Facilitator.ts index daa9836905..0f47ddcb1d 100644 --- a/typescript/packages/core/src/facilitator/x402Facilitator.ts +++ b/typescript/packages/core/src/facilitator/x402Facilitator.ts @@ -372,7 +372,11 @@ export class x402Facilitator { }; for (const hook of this.afterVerifyHooks) { - await hook(resultContext); + try { + await hook(resultContext); + } catch (hookError) { + console.error("afterVerify hook threw an error (payment succeeded):", hookError); + } } return verifyResult; @@ -464,7 +468,11 @@ export class x402Facilitator { }; for (const hook of this.afterSettleHooks) { - await hook(resultContext); + try { + await hook(resultContext); + } catch (hookError) { + console.error("afterSettle hook threw an error (settlement succeeded):", hookError); + } } return settleResult; diff --git a/typescript/packages/core/src/server/x402ResourceServer.ts b/typescript/packages/core/src/server/x402ResourceServer.ts index 14f8aab79e..0af5cbc255 100644 --- a/typescript/packages/core/src/server/x402ResourceServer.ts +++ b/typescript/packages/core/src/server/x402ResourceServer.ts @@ -744,7 +744,11 @@ export class x402ResourceServer { }; for (const hook of this.afterVerifyHooks) { - await hook(resultContext); + try { + await hook(resultContext); + } catch (hookError) { + console.error("afterVerify hook threw an error (payment succeeded):", hookError); + } } return verifyResult; @@ -875,7 +879,11 @@ export class x402ResourceServer { }; for (const hook of this.afterSettleHooks) { - await hook(resultContext); + try { + await hook(resultContext); + } catch (hookError) { + console.error("afterSettle hook threw an error (settlement succeeded):", hookError); + } } // Let declared extensions add data to settlement response diff --git a/typescript/packages/core/test/unit/facilitator/x402Facilitator.hooks.test.ts b/typescript/packages/core/test/unit/facilitator/x402Facilitator.hooks.test.ts index dd3910e493..a7a0d4ae30 100644 --- a/typescript/packages/core/test/unit/facilitator/x402Facilitator.hooks.test.ts +++ b/typescript/packages/core/test/unit/facilitator/x402Facilitator.hooks.test.ts @@ -331,4 +331,51 @@ describe("x402Facilitator - Lifecycle Hooks", () => { expect(result).toBe(facilitator); }); }); + + describe("hook error isolation", () => { + it("should not treat payment as failed when afterVerify hook throws", async () => { + const facilitator = new x402Facilitator(); + facilitator.register("eip155:8453", new MockSchemeFacilitator()); + + facilitator.onAfterVerify(async () => { + throw new Error("Hook side-effect failed"); + }); + + // Payment verification succeeded — hook error must not surface as a verify failure + const result = await facilitator.verify(buildPaymentPayload(), buildPaymentRequirements()); + expect(result.isValid).toBe(true); + }); + + it("should not treat settlement as failed when afterSettle hook throws", async () => { + const facilitator = new x402Facilitator(); + facilitator.register("eip155:8453", new MockSchemeFacilitator()); + + facilitator.onAfterSettle(async () => { + throw new Error("Hook side-effect failed"); + }); + + // Settlement succeeded — hook error must not surface as a settle failure + const result = await facilitator.settle(buildPaymentPayload(), buildPaymentRequirements()); + expect(result.success).toBe(true); + }); + + it("should continue remaining afterVerify hooks after one throws", async () => { + const facilitator = new x402Facilitator(); + facilitator.register("eip155:8453", new MockSchemeFacilitator()); + + let secondHookCalled = false; + + facilitator + .onAfterVerify(async () => { + throw new Error("First hook fails"); + }) + .onAfterVerify(async () => { + secondHookCalled = true; + }); + + await facilitator.verify(buildPaymentPayload(), buildPaymentRequirements()); + expect(secondHookCalled).toBe(true); + }); + }); + }); diff --git a/typescript/packages/core/test/unit/server/x402ResourceServer.test.ts b/typescript/packages/core/test/unit/server/x402ResourceServer.test.ts index 1966efa662..6d67fca490 100644 --- a/typescript/packages/core/test/unit/server/x402ResourceServer.test.ts +++ b/typescript/packages/core/test/unit/server/x402ResourceServer.test.ts @@ -545,6 +545,43 @@ describe("x402ResourceServer", () => { }); }); + describe("hook error isolation", () => { + it("should not treat payment as failed when afterVerify hook throws", async () => { + server.onAfterVerify(async () => { + throw new Error("Hook side-effect failed"); + }); + + // Hook error must not surface as a verify failure + const result = await server.verifyPayment(buildPaymentPayload(), buildPaymentRequirements()); + expect(result.isValid).toBe(true); + }); + + it("should continue remaining afterVerify hooks after one throws", async () => { + let secondHookCalled = false; + + server + .onAfterVerify(async () => { + throw new Error("First hook fails"); + }) + .onAfterVerify(async () => { + secondHookCalled = true; + }); + + await server.verifyPayment(buildPaymentPayload(), buildPaymentRequirements()); + expect(secondHookCalled).toBe(true); + }); + + it("should not treat settlement as failed when afterSettle hook throws", async () => { + server.onAfterSettle(async () => { + throw new Error("Hook side-effect failed"); + }); + + // Hook error must not surface as a settle failure + const result = await server.settlePayment(buildPaymentPayload(), buildPaymentRequirements()); + expect(result.success).toBe(true); + }); + }); + describe("onVerifyFailure", () => { it("should execute when verification fails", async () => { let hookExecuted = false;