fix(payments-plugin): Fix stripe payment transaction handling (#2402)

Implement transactional handling for Stripe Webhook. This is in response to race conditions arising in setups using DB replication. - Wrap all Stripe webhook-related operations inside a single transaction using `TransactionalConnection.withTransaction()`. - Ensures that all database operations within the webhook use the "master" instance. This change aims to solve the issue of database operations in the Stripe webhook not consistently using the master instance, which led to inconsistencies in low-latency environments.
vendure-ecommerce · Sep 19, 2023 · fd8a777 · fd8a777
1 parent 7e2c17a
commit fd8a777
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 110 deletions.
diff --git a/packages/payments-plugin/src/stripe/index.ts b/packages/payments-plugin/src/stripe/index.ts
@@ -1,2 +1 @@
-export * from './stripe.plugin';
-export * from './';
+export { StripePlugin } from './stripe.plugin';
diff --git a/packages/payments-plugin/src/stripe/stripe-client.ts b/packages/payments-plugin/src/stripe/stripe-client.ts
@@ -6,7 +6,7 @@ import Stripe from 'stripe';
 export class VendureStripeClient extends Stripe {
     constructor(private apiKey: string, public webhookSecret: string) {
         super(apiKey, {
-            apiVersion: null as any, // Use accounts default version
+            apiVersion: null as unknown as Stripe.LatestApiVersion, // Use accounts default version
         });
     }
 }
diff --git a/packages/payments-plugin/src/stripe/stripe-utils.ts b/packages/payments-plugin/src/stripe/stripe-utils.ts
@@ -12,10 +12,7 @@ import { CurrencyCode, Order } from '@vendure/core';
  * stores money amounts multiplied by 100). See https://github.com/vendure-ecommerce/vendure/issues/1630
  */
 export function getAmountInStripeMinorUnits(order: Order): number {
-    const amountInStripeMinorUnits = currencyHasFractionPart(order.currencyCode)
-        ? order.totalWithTax
-        : Math.round(order.totalWithTax / 100);
-    return amountInStripeMinorUnits;
+    return currencyHasFractionPart(order.currencyCode) ? order.totalWithTax : Math.round(order.totalWithTax / 100);
 }
 
 /**
@@ -24,10 +21,7 @@ export function getAmountInStripeMinorUnits(order: Order): number {
  * used by Vendure.
  */
 export function getAmountFromStripeMinorUnits(order: Order, stripeAmount: number): number {
-    const amountInVendureMinorUnits = currencyHasFractionPart(order.currencyCode)
-        ? stripeAmount
-        : stripeAmount * 100;
-    return amountInVendureMinorUnits;
+    return currencyHasFractionPart(order.currencyCode) ? stripeAmount : stripeAmount * 100;
 }
 
 function currencyHasFractionPart(currencyCode: CurrencyCode): boolean {
@@ -36,6 +30,6 @@ function currencyHasFractionPart(currencyCode: CurrencyCode): boolean {
         currency: currencyCode,
         currencyDisplay: 'symbol',
     }).formatToParts(123.45);
-    const hasFractionPart = !!parts.find(p => p.type === 'fraction');
-    return hasFractionPart;
+
+    return !!parts.find(p => p.type === 'fraction');
 }
diff --git a/packages/payments-plugin/src/stripe/stripe.controller.ts b/packages/payments-plugin/src/stripe/stripe.controller.ts
@@ -1,18 +1,9 @@
 import { Controller, Headers, HttpStatus, Post, Req, Res } from '@nestjs/common';
-import {
-    InternalServerError,
-    LanguageCode,
-    Logger,
-    Order,
-    OrderService,
-    PaymentMethod,
-    PaymentMethodService,
-    RequestContext,
-    RequestContextService,
-} from '@vendure/core';
+import type { PaymentMethod, RequestContext } from '@vendure/core';
+import { InternalServerError, LanguageCode, Logger, Order, OrderService, PaymentMethodService, RequestContextService, TransactionalConnection } from '@vendure/core';
 import { OrderStateTransitionError } from '@vendure/core/dist/common/error/generated-graphql-shop-errors';
-import { Response } from 'express';
-import Stripe from 'stripe';
+import type { Response } from 'express';
+import type Stripe from 'stripe';
 
 import { loggerCtx } from './constants';
 import { stripePaymentMethodHandler } from './stripe.handler';
@@ -30,6 +21,7 @@ export class StripeController {
         private orderService: OrderService,
         private stripeService: StripeService,
         private requestContextService: RequestContextService,
+        private connection: TransactionalConnection,
     ) {}
 
     @Post('stripe')
@@ -43,72 +35,76 @@ export class StripeController {
             response.status(HttpStatus.BAD_REQUEST).send(missingHeaderErrorMessage);
             return;
         }
+
         const event = request.body as Stripe.Event;
         const paymentIntent = event.data.object as Stripe.PaymentIntent;
+
         if (!paymentIntent) {
             Logger.error(noPaymentIntentErrorMessage, loggerCtx);
             response.status(HttpStatus.BAD_REQUEST).send(noPaymentIntentErrorMessage);
             return;
         }
+
         const { metadata: { channelToken, orderCode, orderId } = {} } = paymentIntent;
-        const ctx = await this.createContext(channelToken, request);
-        const order = await this.orderService.findOneByCode(ctx, orderCode);
-        if (!order) {
-            throw Error(`Unable to find order ${orderCode}, unable to settle payment ${paymentIntent.id}!`);
-        }
-        try {
-            // Throws an error if the signature is invalid
-            await this.stripeService.constructEventFromPayload(ctx, order, request.rawBody, signature);
-        } catch (e: any) {
-            Logger.error(`${signatureErrorMessage} ${signature}: ${(e as Error)?.message}`, loggerCtx);
-            response.status(HttpStatus.BAD_REQUEST).send(signatureErrorMessage);
-            return;
-        }
-        if (event.type === 'payment_intent.payment_failed') {
-            const message = paymentIntent.last_payment_error?.message ?? 'unknown error';
-            Logger.warn(`Payment for order ${orderCode} failed: ${message}`, loggerCtx);
-            response.status(HttpStatus.OK).send('Ok');
-            return;
-        }
-        if (event.type !== 'payment_intent.succeeded') {
-            // This should never happen as the webhook is configured to receive
-            // payment_intent.succeeded and payment_intent.payment_failed events only
-            Logger.info(`Received ${event.type} status update for order ${orderCode}`, loggerCtx);
-            return;
-        }
-        if (order.state !== 'ArrangingPayment') {
-            const transitionToStateResult = await this.orderService.transitionToState(
-                ctx,
-                orderId,
-                'ArrangingPayment',
-            );
-
-            if (transitionToStateResult instanceof OrderStateTransitionError) {
-                Logger.error(
-                    `Error transitioning order ${orderCode} to ArrangingPayment state: ${transitionToStateResult.message}`,
-                    loggerCtx,
-                );
+        const outerCtx = await this.createContext(channelToken, request);
+
+        await this.connection.withTransaction(outerCtx, async (ctx) => {
+            const order = await this.orderService.findOneByCode(ctx, orderCode);
+
+            if (!order) {
+                throw new Error(`Unable to find order ${orderCode}, unable to settle payment ${paymentIntent.id}!`);
+            }
+
+            try {
+                // Throws an error if the signature is invalid
+                await this.stripeService.constructEventFromPayload(ctx, order, request.rawBody, signature);
+            } catch (e: any) {
+                Logger.error(`${signatureErrorMessage} ${signature}: ${(e as Error)?.message}`, loggerCtx);
+                response.status(HttpStatus.BAD_REQUEST).send(signatureErrorMessage);
                 return;
             }
-        }
 
-        const paymentMethod = await this.getPaymentMethod(ctx);
+            if (event.type === 'payment_intent.payment_failed') {
+                const message = paymentIntent.last_payment_error?.message ?? 'unknown error';
+                Logger.warn(`Payment for order ${orderCode} failed: ${message}`, loggerCtx);
+                response.status(HttpStatus.OK).send('Ok');
+                return;
+            }
 
-        const addPaymentToOrderResult = await this.orderService.addPaymentToOrder(ctx, orderId, {
-            method: paymentMethod.code,
-            metadata: {
-                paymentIntentAmountReceived: paymentIntent.amount_received,
-                paymentIntentId: paymentIntent.id,
-            },
-        });
+            if (event.type !== 'payment_intent.succeeded') {
+                // This should never happen as the webhook is configured to receive
+                // payment_intent.succeeded and payment_intent.payment_failed events only
+                Logger.info(`Received ${event.type} status update for order ${orderCode}`, loggerCtx);
+                return;
+            }
 
-        if (!(addPaymentToOrderResult instanceof Order)) {
-            Logger.error(
-                `Error adding payment to order ${orderCode}: ${addPaymentToOrderResult.message}`,
-                loggerCtx,
-            );
-            return;
-        }
+            if (order.state !== 'ArrangingPayment') {
+                const transitionToStateResult = await this.orderService.transitionToState(
+                    ctx,
+                    orderId,
+                    'ArrangingPayment',
+                );
+
+                if (transitionToStateResult instanceof OrderStateTransitionError) {
+                    Logger.error(`Error transitioning order ${orderCode} to ArrangingPayment state: ${transitionToStateResult.message}`, loggerCtx);
+                    return;
+                }
+            }
+
+            const paymentMethod = await this.getPaymentMethod(ctx);
+
+            const addPaymentToOrderResult = await this.orderService.addPaymentToOrder(ctx, orderId, {
+                method: paymentMethod.code,
+                metadata: {
+                    paymentIntentAmountReceived: paymentIntent.amount_received,
+                    paymentIntentId: paymentIntent.id,
+                },
+            });
+
+            if (!(addPaymentToOrderResult instanceof Order)) {
+                Logger.error(`Error adding payment to order ${orderCode}: ${addPaymentToOrderResult.message}`, loggerCtx);
+            }
+        });
 
         Logger.info(`Stripe payment intent id ${paymentIntent.id} added to order ${orderCode}`, loggerCtx);
         response.status(HttpStatus.OK).send('Ok');
@@ -127,8 +123,9 @@ export class StripeController {
         const method = (await this.paymentMethodService.findAll(ctx)).items.find(
             m => m.handler.code === stripePaymentMethodHandler.code,
         );
+
         if (!method) {
-            throw new InternalServerError(`[${loggerCtx}] Could not find Stripe PaymentMethod`);
+          throw new InternalServerError(`[${loggerCtx}] Could not find Stripe PaymentMethod`);
         }
 
         return method;

diff --git a/packages/payments-plugin/src/stripe/stripe.resolver.ts b/packages/payments-plugin/src/stripe/stripe.resolver.ts
@@ -1,19 +1,14 @@
 import { Mutation, Resolver } from '@nestjs/graphql';
-import {
-    ActiveOrderService,
-    Allow,
-    Ctx,
-    Permission,
-    RequestContext,
-    UnauthorizedError,
-    UserInputError,
-} from '@vendure/core';
+import { ActiveOrderService, Allow, Ctx, Permission, RequestContext, UnauthorizedError, UserInputError } from '@vendure/core';
 
 import { StripeService } from './stripe.service';
 
 @Resolver()
 export class StripeResolver {
-    constructor(private stripeService: StripeService, private activeOrderService: ActiveOrderService) {}
+    constructor(
+        private stripeService: StripeService,
+        private activeOrderService: ActiveOrderService,
+    ) {}
 
     @Mutation()
     @Allow(Permission.Owner)

diff --git a/packages/payments-plugin/src/stripe/stripe.service.ts b/packages/payments-plugin/src/stripe/stripe.service.ts
@@ -1,18 +1,7 @@
 import { Inject, Injectable } from '@nestjs/common';
 import { ModuleRef } from '@nestjs/core';
 import { ConfigArg } from '@vendure/common/lib/generated-types';
-import {
-    Ctx,
-    Customer,
-    Injector,
-    Logger,
-    Order,
-    Payment,
-    PaymentMethodService,
-    RequestContext,
-    TransactionalConnection,
-    UserInputError,
-} from '@vendure/core';
+import { Customer, Injector, Logger, Order, Payment, PaymentMethodService, RequestContext, TransactionalConnection, UserInputError } from '@vendure/core';
 import Stripe from 'stripe';
 
 import { loggerCtx, STRIPE_PLUGIN_OPTIONS } from './constants';
@@ -25,9 +14,9 @@ import { StripePluginOptions } from './types';
 @Injectable()
 export class StripeService {
     constructor(
+        @Inject(STRIPE_PLUGIN_OPTIONS) private options: StripePluginOptions,
         private connection: TransactionalConnection,
         private paymentMethodService: PaymentMethodService,
-        @Inject(STRIPE_PLUGIN_OPTIONS) private options: StripePluginOptions,
         private moduleRef: ModuleRef,
     ) {}
 
@@ -64,10 +53,7 @@ export class StripeService {
 
         if (!client_secret) {
             // This should never happen
-            Logger.warn(
-                `Payment intent creation for order ${order.code} did not return client secret`,
-                loggerCtx,
-            );
+            Logger.warn(`Payment intent creation for order ${order.code} did not return client secret`, loggerCtx);
             throw Error('Failed to create payment intent');
         }
 

diff --git a/packages/payments-plugin/src/stripe/types.ts b/packages/payments-plugin/src/stripe/types.ts
@@ -1,7 +1,7 @@
-import { Injector, Order, RequestContext } from '@vendure/core';
 import '@vendure/core/dist/entity/custom-entity-fields';
-import { Request } from 'express';
-import Stripe from 'stripe';
+import type { Injector, Order, RequestContext } from '@vendure/core';
+import type { Request } from 'express';
+import type Stripe from 'stripe';
 
 // Note: deep import is necessary here because CustomCustomerFields is also extended in the Braintree
 // plugin. Reference: https://github.com/microsoft/TypeScript/issues/46617