From 7df72676c90bae4839d1f24c0934426b9129bccb Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:55:06 +0200 Subject: [PATCH] fix(paypal): replace a broken SDK client and time out after 15 s The PayPal SDK keeps the promise of its OAuth token for the lifetime of its Client, also when the token request failed. After one failed token request, as during the DNS outage, every later call failed with that same error and without a request until the process restarted: once DNS was back, the investigators' harness (scenario S7) saw 0 DNS lookups. The client had no timeout either: the SDK's default of 0 means none. PayPalApi now reaches PayPal through createPayPalClient (src/checkout/paypal-client.ts). It creates the SDK's Client and OrdersController through an injected factory on the first call, and anew after an error of the token request, an error without an answer from PayPal (network, DNS, timeout), 401 or 5xx. PayPal's other answers, such as 422, keep the client. A call that fails on a client that was replaced meanwhile leaves its successor alone. A replacement is logged as a warning with the error described. The client times out after 15 s. paypalClientConfiguration builds its configuration, with the logging of bodies and headers off. The tests use a fake factory that, like the SDK, keeps a failed token request per client, and count constructions and token requests. With the real SDK in the harness, S7 looks DNS up again and every Client is created with a timeout of 15000. Refs https://git.librete.ch/libretech/mp/issues/71 Refs https://git.librete.ch/libretech/mp/issues/67 --- src/checkout/paypal-client.ts | 96 +++++++++++++++++ src/services/PayPalApi.ts | 37 +++---- tests/unit/paypal-client.test.ts | 171 +++++++++++++++++++++++++++++++ 3 files changed, 283 insertions(+), 21 deletions(-) create mode 100644 src/checkout/paypal-client.ts create mode 100644 tests/unit/paypal-client.test.ts diff --git a/src/checkout/paypal-client.ts b/src/checkout/paypal-client.ts new file mode 100644 index 0000000..901368f --- /dev/null +++ b/src/checkout/paypal-client.ts @@ -0,0 +1,96 @@ +import type { Configuration, Environment } from "@paypal/paypal-server-sdk"; + +// The PayPal SDK client behind PayPalApi, created through a factory so that it can be replaced: PayPalApi passes one that creates +// the SDK's Client and OrdersController, the tests a fake. +// +// The SDK keeps the promise of its OAuth token for the lifetime of a Client, also when the token request failed: every later call +// awaits the same rejected promise and fails with the same error without a request, until the process restarts. One failed token +// request, during a DNS outage for example, stopped every checkout until a restart. + +/** The SDK's default timeout is 0, which means none: a call to PayPal that hangs would hang the checkout with it. */ +export const PAYPAL_TIMEOUT_MS = 15000; + +/** + * The configuration of the SDK's Client. The SDK logs method, URL and status of each request to the console, but no bodies, since + * a request body holds the buyer's name and address, and no headers, since they hold the token. + */ +export const paypalClientConfiguration = ({ + clientId, + clientSecret, + environment +}: { + clientId: string; + clientSecret: string; + environment: Environment; +}): Partial => ({ + clientCredentialsAuthCredentials: { + oAuthClientId: clientId, + oAuthClientSecret: clientSecret + }, + timeout: PAYPAL_TIMEOUT_MS, + environment, + logging: { + logRequest: { logBody: false, logHeaders: false }, + logResponse: { logBody: false, logHeaders: false } + } +}); + +const TOKEN_PATH = "/v1/oauth2/token"; + +/** The URL of the request the error belongs to: an error of the SDK carries the request, an axios error its config. */ +const requestUrl = (error: object): string => { + const { request, config } = error as { request?: { url?: unknown }; config?: { url?: unknown } }; + const url = request?.url ?? config?.url; + return typeof url === "string" ? url : ""; +}; + +/** + * Whether the error may leave the client unusable, so that the next call needs a new one: any error of the token request, which + * the SDK keeps; an error without an answer from PayPal (network, DNS, timeout); 401, a token PayPal no longer accepts; and 5xx. + * PayPal's other answers, such as 422 for an order it rejects, leave the client as it is. + */ +export const breaksClient = (error: unknown): boolean => { + if (typeof error !== "object" || error === null) return true; + const { statusCode } = error as { statusCode?: unknown }; + if (typeof statusCode !== "number") return true; + if (statusCode === 401 || statusCode >= 500) return true; + return requestUrl(error).includes(TOKEN_PATH); +}; + +export type PayPalClient = { + /** Runs the request on the current controller. An error that breaksClient replaces the controller for the calls after it. */ + call(request: (controller: C) => Promise): Promise; +}; + +/** + * Creates the controller on the first call, and a new one, with a new client and token, on the first call after an error that + * breaksClient. onReplace is called with that error. + */ +export const createPayPalClient = ({ + configuration, + createController, + onReplace +}: { + configuration: Partial; + createController: (configuration: Partial) => C; + onReplace?: (error: unknown) => void; +}): PayPalClient => { + let current: C | undefined; + + return { + async call(request: (controller: C) => Promise): Promise { + current ??= createController(configuration); + const controller = current; + try { + return await request(controller); + } catch (error) { + // A call that failed on a controller which was replaced meanwhile leaves its successor alone. + if (breaksClient(error) && current === controller) { + current = undefined; + onReplace?.(error); + } + throw error; + } + } + }; +}; diff --git a/src/services/PayPalApi.ts b/src/services/PayPalApi.ts index e5f2a15..8951a57 100644 --- a/src/services/PayPalApi.ts +++ b/src/services/PayPalApi.ts @@ -1,30 +1,23 @@ -import { Client, Environment, LogLevel, OrdersController, OrderRequest } from "@paypal/paypal-server-sdk"; +import { Client, OrdersController } from "@paypal/paypal-server-sdk"; +import type { Environment, OrderRequest } from "@paypal/paypal-server-sdk"; import { paypalClientId, paypalClientSecret, paypalEnvironment } from "../../config/constants"; +import { createPayPalClient, paypalClientConfiguration } from "../checkout/paypal-client"; +import type { PayPalClient } from "../checkout/paypal-client"; import { buildPayPalOrderRequest } from "../checkout/paypal-order"; import type { PayPalOrderSource } from "../checkout/paypal-order"; import { describeError } from "../logging/error"; class PayPalApi { - private readonly client: Client; - private readonly ordersController: OrdersController; + // A new SDK Client and OrdersController after a failed token request, a network error, 401 or 5xx: see src/checkout/paypal-client.ts. + private readonly orders: PayPalClient; constructor(clientId: string, clientSecret: string, environment: Environment) { - this.client = new Client({ - clientCredentialsAuthCredentials: { - oAuthClientId: clientId, - oAuthClientSecret: clientSecret - }, - timeout: 0, - environment, - // The SDK logs method, URL and status of each request to the console. Not the bodies: a request body holds the buyer's name - // and address. Not the headers: they hold the token. - logging: { - logLevel: LogLevel.Info, - logRequest: { logBody: false, logHeaders: false }, - logResponse: { logBody: false, logHeaders: false } - } + this.orders = createPayPalClient({ + configuration: paypalClientConfiguration({ clientId, clientSecret, environment }), + createController: (configuration) => new OrdersController(new Client(configuration)), + onReplace: (error) => + strapi.log.warn("app:w:paypal-api: Replacing the PayPal client after a failed request", { error: describeError(error) }) }); - this.ordersController = new OrdersController(this.client); } /** @@ -36,7 +29,7 @@ class PayPalApi { const request = buildPayPalOrderRequest(returnUrl, order) as OrderRequest; try { - const { body } = await this.ordersController.ordersCreate({ body: request, prefer: "return=minimal" }); + const { body } = await this.orders.call((orders) => orders.ordersCreate({ body: request, prefer: "return=minimal" })); const session = JSON.parse(body as string); if (typeof session?.id !== "string" || session.id === "") throw new Error("PayPal answered without an order id"); @@ -56,7 +49,7 @@ class PayPalApi { /** PayPal's order as parsed JSON in PayPal's snake_case (GET /v2/checkout/orders/:id). Throws PayPal's and network errors. */ async getOrder(id: string): Promise { - const { body } = await this.ordersController.ordersGet({ id }); + const { body } = await this.orders.call((orders) => orders.ordersGet({ id })); return JSON.parse(body as string); } @@ -66,7 +59,9 @@ class PayPalApi { * response of the first capture instead of capturing again. Throws PayPal's and network errors. */ async captureOrder(id: string, requestId: string): Promise { - const { body } = await this.ordersController.ordersCapture({ id, paypalRequestId: requestId, prefer: "return=representation" }); + const { body } = await this.orders.call((orders) => + orders.ordersCapture({ id, paypalRequestId: requestId, prefer: "return=representation" }) + ); return JSON.parse(body as string); } diff --git a/tests/unit/paypal-client.test.ts b/tests/unit/paypal-client.test.ts new file mode 100644 index 0000000..4e9e53a --- /dev/null +++ b/tests/unit/paypal-client.test.ts @@ -0,0 +1,171 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { PAYPAL_TIMEOUT_MS, breaksClient, createPayPalClient, paypalClientConfiguration } from "../../src/checkout/paypal-client.ts"; + +const TOKEN_URL = "https://api-m.paypal.com/v1/oauth2/token"; +const ORDERS_URL = "https://api-m.paypal.com/v2/checkout/orders"; +const PAYPAL_ORDER = '{"id":"5O190127TN364715T","status":"CREATED"}'; + +// What axios throws when PayPal cannot be reached: no status code, the request in config. +const networkError = (url = TOKEN_URL) => + Object.assign(new Error("getaddrinfo EAI_AGAIN api-m.paypal.com"), { name: "AxiosError", code: "EAI_AGAIN", config: { method: "post", url } }); + +// What the SDK throws when PayPal answers with an error status (@apimatic/core's ApiError): the status code and the request. +const apiError = (statusCode: number, url = ORDERS_URL) => + Object.assign(new Error(`PayPal answered ${statusCode}`), { statusCode, request: { method: "POST", url }, headers: {}, body: "{}" }); + +/** + * A fake of the SDK's OrdersController over a Client. Like the SDK, every client requests its OAuth token on its first call and + * keeps the promise for its lifetime, also when the token request failed: every later call fails with that same error and + * requests nothing. The token request fails while dns.up is false. Each construction and token request is counted. + */ +const fakeSdk = () => { + const sdk = { dnsUp: false, constructions: 0, tokenRequests: 0, configurations: [] as unknown[] }; + const createController = (configuration: unknown) => { + sdk.constructions += 1; + sdk.configurations.push(configuration); + let token: Promise | undefined; + const authenticate = () => { + if (token === undefined) { + sdk.tokenRequests += 1; + token = sdk.dnsUp ? Promise.resolve("A21AAF…") : Promise.reject(networkError()); + token.catch(() => {}); + } + return token; + }; + return { + ordersCreate: async () => { + await authenticate(); + return { statusCode: 201, body: PAYPAL_ORDER }; + } + }; + }; + return { sdk, createController }; +}; + +test("a client of the SDK keeps a failed token request: every later call fails without a request", async () => { + // What the replacement below works around, as the harness's scenario S7 observed with the real SDK. + const { sdk, createController } = fakeSdk(); + const orders = createController({}); + + const first = await orders.ordersCreate().catch((error) => error); + sdk.dnsUp = true; + const second = await orders.ordersCreate().catch((error) => error); + + assert.equal(second, first); + assert.equal(sdk.tokenRequests, 1); +}); + +test("replaces the client after a failed token request, so that the next call requests a new token", async () => { + const { sdk, createController } = fakeSdk(); + const replaced: unknown[] = []; + const client = createPayPalClient({ configuration: {}, createController, onReplace: (error) => replaced.push(error) }); + + await assert.rejects( + client.call((orders) => orders.ordersCreate()), + { name: "AxiosError", message: "getaddrinfo EAI_AGAIN api-m.paypal.com" } + ); + assert.deepEqual([sdk.constructions, sdk.tokenRequests, replaced.length], [1, 1, 1]); + + sdk.dnsUp = true; + assert.deepEqual(await client.call((orders) => orders.ordersCreate()), { statusCode: 201, body: PAYPAL_ORDER }); + assert.deepEqual([sdk.constructions, sdk.tokenRequests], [2, 2]); + + // The new client keeps its token. + await client.call((orders) => orders.ordersCreate()); + assert.deepEqual([sdk.constructions, sdk.tokenRequests, replaced.length], [2, 2, 1]); +}); + +test("keeps the client after an answer of PayPal that leaves it usable", async () => { + let constructions = 0; + const client = createPayPalClient({ + configuration: {}, + createController: () => { + constructions += 1; + return { ordersCreate: async () => Promise.reject(apiError(422)) }; + } + }); + + for (let call = 0; call < 3; call++) { + const error = await client.call((orders) => orders.ordersCreate()).catch((rejection) => rejection); + assert.equal(error.statusCode, 422); + } + assert.equal(constructions, 1); +}); + +test("a call that fails on a replaced client does not replace its successor", async () => { + let constructions = 0; + const pending: ((error: unknown) => void)[] = []; + const replaced: unknown[] = []; + const client = createPayPalClient({ + configuration: {}, + createController: () => { + constructions += 1; + return { ordersGet: () => new Promise((_, reject) => pending.push(reject)) }; + }, + onReplace: (error) => replaced.push(error) + }); + + // Two calls run on the first client. The first one fails, which replaces the client, and a third call starts on the second. + const first = client.call((orders) => orders.ordersGet()); + const second = client.call((orders) => orders.ordersGet()); + pending[0](networkError(ORDERS_URL)); + await assert.rejects(first); + const third = client.call((orders) => orders.ordersGet()); + assert.equal(constructions, 2); + + // The second call fails on the first client only now: the second client stays. + pending[1](networkError(ORDERS_URL)); + await assert.rejects(second); + const fourth = client.call((orders) => orders.ordersGet()); + assert.deepEqual([constructions, replaced.length], [2, 1]); + + pending[2](apiError(404)); + pending[3](apiError(404)); + await Promise.all([assert.rejects(third), assert.rejects(fourth)]); +}); + +test("breaks the client with an error of the token request, without an answer, with 401 and with 5xx", () => { + for (const error of [ + networkError(TOKEN_URL), + networkError(ORDERS_URL), + Object.assign(new Error("timeout of 15000ms exceeded"), { name: "AxiosError", code: "ECONNABORTED", config: { url: ORDERS_URL } }), + apiError(400, TOKEN_URL), + apiError(429, TOKEN_URL), + apiError(401, TOKEN_URL), + apiError(401, ORDERS_URL), + apiError(500, ORDERS_URL), + apiError(503, ORDERS_URL), + // The SDK's ResponseValidationError carries the status of the answer it could not read. + Object.assign(new Error("The response did not match the response schema."), { statusCode: 200, request: { url: TOKEN_URL } }), + new Error("The HTTP call was aborted."), + "a string", + undefined + ]) { + assert.equal(breaksClient(error), true, String(error?.message ?? error)); + } + for (const error of [apiError(400), apiError(404), apiError(409), apiError(422), apiError(429)]) { + assert.equal(breaksClient(error), false, error.message); + } +}); + +test("configures the SDK client with a timeout of 15 s and without logging bodies or headers", async () => { + const configuration = paypalClientConfiguration({ clientId: "DUMMY_CLIENT_ID", clientSecret: "DUMMY_CLIENT_SECRET", environment: "Production" }); + + assert.equal(PAYPAL_TIMEOUT_MS, 15000); + assert.deepEqual(configuration, { + clientCredentialsAuthCredentials: { oAuthClientId: "DUMMY_CLIENT_ID", oAuthClientSecret: "DUMMY_CLIENT_SECRET" }, + timeout: 15000, + environment: "Production", + logging: { logRequest: { logBody: false, logHeaders: false }, logResponse: { logBody: false, logHeaders: false } } + }); + + // Every client the factory creates, also after a replacement, gets this configuration. + const { sdk, createController } = fakeSdk(); + const client = createPayPalClient({ configuration, createController }); + await client.call((orders) => orders.ordersCreate()).catch(() => {}); + sdk.dnsUp = true; + await client.call((orders) => orders.ordersCreate()); + assert.equal(sdk.constructions, 2); + for (const passed of sdk.configurations) assert.equal((passed as { timeout: number }).timeout, 15000); +});