feat: Improvements to payment module and Stripe provider (#10980)

* fix: Correctly parse Stripe error, remove unused method

* fix: Isolate the payment provider error check function

* fix: Allow passing few extra parameters to Stripe

---------

Co-authored-by: Oli Juhl <59018053+olivermrbl@users.noreply.github.com>

packages/core/utils/src/payment/abstract-payment-provider.ts

@@ -39,13 +39,13 @@ export abstract class AbstractPaymentProvider<TConfig = Record<string, unknown>>
   static validateOptions(options: Record<any, any>): void | never {}
 
   /**
-   * The constructor allows you to access resources from the [module's container](https://docs.medusajs.com/learn/fundamentals/modules/container) 
+   * The constructor allows you to access resources from the [module's container](https://docs.medusajs.com/learn/fundamentals/modules/container)
    * using the first parameter, and the module's options using the second parameter.
-   * 
+   *
    * :::note
-   * 
+   *
    * A module's options are passed when you register it in the Medusa application.
-   * 
+   *
    * :::
    *
    * @param {Record<string, unknown>} cradle - The module's container cradle used to resolve resources.
@@ -55,35 +55,35 @@ export abstract class AbstractPaymentProvider<TConfig = Record<string, unknown>>
    * @example
    * import { AbstractPaymentProvider } from "@medusajs/framework/utils"
    * import { Logger } from "@medusajs/framework/types"
-   * 
+   *
    * type Options = {
    *   apiKey: string
    * }
-   * 
+   *
    * type InjectedDependencies = {
    *   logger: Logger
    * }
-   * 
+   *
    * class MyPaymentProviderService extends AbstractPaymentProvider<Options> {
    *   protected logger_: Logger
    *   protected options_: Options
    *   // assuming you're initializing a client
    *   protected client
-   * 
+   *
    *   constructor(
    *     container: InjectedDependencies,
    *     options: Options
    *   ) {
    *     super(container, options)
-   * 
+   *
    *     this.logger_ = container.logger
    *     this.options_ = options
-   * 
+   *
    *     // TODO initialize your client
    *   }
    *   // ...
    * }
-   * 
+   *
    * export default MyPaymentProviderService
    */
   protected constructor(
@@ -697,16 +697,3 @@ export abstract class AbstractPaymentProvider<TConfig = Record<string, unknown>>
     data: ProviderWebhookPayload["payload"]
   ): Promise<WebhookActionResult>
 }
-
-/**
- * @ignore
- */
-export function isPaymentProviderError(obj: any): obj is PaymentProviderError {
-  return (
-    obj &&
-    typeof obj === "object" &&
-    "error" in obj &&
-    "code" in obj &&
-    "detail" in obj
-  )
-}

packages/modules/payment/integration-tests/__tests__/services/payment-module/index.spec.ts

@@ -556,7 +556,6 @@ moduleIntegrationTestRunner<IPaymentModuleService>({
               data: {},
               context: {
                 extra: {},
-                resource_id: "test",
                 email: "test@test.com",
                 billing_address: {},
                 customer: {},

packages/modules/payment/src/services/payment-provider.ts

@@ -13,11 +13,7 @@ import {
   UpdatePaymentProviderSession,
   WebhookActionResult,
 } from "@medusajs/framework/types"
-import {
-  isPaymentProviderError,
-  MedusaError,
-  ModulesSdkUtils,
-} from "@medusajs/framework/utils"
+import { MedusaError, ModulesSdkUtils } from "@medusajs/framework/utils"
 import { PaymentProvider } from "@models"
 import { EOL } from "os"
 
@@ -171,3 +167,13 @@ Please make sure that the provider is registered in the container and it is conf
     )
   }
 }
+
+function isPaymentProviderError(obj: any): obj is PaymentProviderError {
+  return (
+    obj &&
+    typeof obj === "object" &&
+    "error" in obj &&
+    "code" in obj &&
+    "detail" in obj
+  )
+}

packages/modules/providers/payment-stripe/src/core/stripe-base.ts

@@ -1,5 +1,3 @@
-import { EOL } from "os"
-
 import Stripe from "stripe"
 
 import {
@@ -13,9 +11,7 @@ import {
 import {
   AbstractPaymentProvider,
   isDefined,
-  isPaymentProviderError,
   isPresent,
-  MedusaError,
   PaymentActions,
   PaymentSessionStatus,
 } from "@medusajs/framework/utils"
@@ -60,23 +56,37 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
     return this.options_
   }
 
-  getPaymentIntentOptions(): PaymentIntentOptions {
-    const options: PaymentIntentOptions = {}
+  normalizePaymentIntentParameters(
+    extra?: Record<string, unknown>
+  ): Partial<Stripe.PaymentIntentCreateParams> {
+    const res = {} as Partial<Stripe.PaymentIntentCreateParams>
 
-    if (this?.paymentIntentOptions?.capture_method) {
-      options.capture_method = this.paymentIntentOptions.capture_method
-    }
+    res.description = (extra?.payment_description ??
+      this.options_?.paymentDescription) as string
 
-    if (this?.paymentIntentOptions?.setup_future_usage) {
-      options.setup_future_usage = this.paymentIntentOptions.setup_future_usage
-    }
+    res.capture_method =
+      (extra?.capture_method as "automatic" | "manual") ??
+      this.paymentIntentOptions.capture_method ??
+      (this.options_.capture ? "automatic" : "manual")
 
-    if (this?.paymentIntentOptions?.payment_method_types) {
-      options.payment_method_types =
-        this.paymentIntentOptions.payment_method_types
-    }
+    res.setup_future_usage =
+      (extra?.setup_future_usage as "off_session" | "on_session" | undefined) ??
+      this.paymentIntentOptions.setup_future_usage
 
-    return options
+    res.payment_method_types = this.paymentIntentOptions
+      .payment_method_types as string[]
+
+    res.automatic_payment_methods =
+      (extra?.automatic_payment_methods as { enabled: true } | undefined) ??
+      (this.options_?.automaticPaymentMethods ? { enabled: true } : undefined)
+
+    res.off_session = extra?.off_session as boolean | undefined
+
+    res.confirm = extra?.confirm as boolean | undefined
+
+    res.payment_method = extra?.payment_method as string | undefined
+
+    return res
   }
 
   async getPaymentStatus(
@@ -106,24 +116,16 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
   async initiatePayment(
     input: CreatePaymentProviderSession
   ): Promise<PaymentProviderError | PaymentProviderSessionResponse> {
-    const intentRequestData = this.getPaymentIntentOptions()
     const { email, extra, session_id, customer } = input.context
     const { currency_code, amount } = input
 
-    const description = (extra?.payment_description ??
-      this.options_?.paymentDescription) as string
+    const additionalParameters = this.normalizePaymentIntentParameters(extra)
 
     const intentRequest: Stripe.PaymentIntentCreateParams = {
-      description,
       amount: getSmallestUnit(amount, currency_code),
       currency: currency_code,
       metadata: { session_id: session_id! },
-      capture_method: this.options_.capture ? "automatic" : "manual",
-      ...intentRequestData,
-    }
-
-    if (this.options_?.automaticPaymentMethods) {
-      intentRequest.automatic_payment_methods = { enabled: true }
+      ...additionalParameters,
     }
 
     if (customer?.metadata?.stripe_id) {
@@ -273,15 +275,7 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
     const stripeId = context.customer?.metadata?.stripe_id
 
     if (stripeId !== data.customer) {
-      const result = await this.initiatePayment(input)
-      if (isPaymentProviderError(result)) {
-        return this.buildError(
-          "An error occurred in updatePayment during the initiate of the new payment for the new customer",
-          result
-        )
-      }
-
-      return result
+      return await this.initiatePayment(input)
     } else {
       if (isPresent(amount) && data.amount === amountNumeric) {
         return { data }
@@ -300,25 +294,6 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
     }
   }
 
-  async updatePaymentData(sessionId: string, data: Record<string, unknown>) {
-    try {
-      // Prevent from updating the amount from here as it should go through
-      // the updatePayment method to perform the correct logic
-      if (isPresent(data.amount)) {
-        throw new MedusaError(
-          MedusaError.Types.INVALID_DATA,
-          "Cannot update amount, use updatePayment instead"
-        )
-      }
-
-      return (await this.stripe_.paymentIntents.update(sessionId, {
-        ...data,
-      })) as unknown as PaymentProviderSessionResponse["data"]
-    } catch (e) {
-      return this.buildError("An error occurred in updatePaymentData", e)
-    }
-  }
-
   async getWebhookActionAndData(
     webhookData: ProviderWebhookPayload["payload"]
   ): Promise<WebhookActionResult> {
@@ -374,18 +349,17 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
       this.options_.webhookSecret
     )
   }
-  protected buildError(
-    message: string,
-    error: Stripe.StripeRawError | PaymentProviderError | Error
-  ): PaymentProviderError {
+  protected buildError(message: string, error: Error): PaymentProviderError {
+    const errorDetails =
+      "raw" in error ? (error.raw as Stripe.StripeRawError) : error
+
     return {
-      error: message,
-      code: "code" in error ? error.code : "unknown",
-      detail: isPaymentProviderError(error)
-        ? `${error.error}${EOL}${error.detail ?? ""}`
-        : "detail" in error
-        ? error.detail
-        : error.message ?? "",
+      error: `${message}: ${error.message}`,
+      code: "code" in errorDetails ? errorDetails.code : "unknown",
+      detail:
+        "detail" in errorDetails
+          ? `${error.message}: ${errorDetails.detail}`
+          : error.message,
     }
   }
 }