refactor(chore): address PR review comments for type safety and error handling

address PR review comments for type safety and error handling

GH-21

Author: Piyush Singh Gaur

src/providers/billing.provider.ts

@@ -1,14 +1,7 @@
 import {inject, Provider} from '@loopback/core';
-
 import {HttpErrors} from '@loopback/rest';
 import {BillingComponentBindings} from '../keys';
-import {
-  IService,
-  TCustomer,
-  TInvoice,
-  TPaymentSource,
-  Transaction,
-} from '../types';
+import {IService} from '../types';
 
 export class BillingProvider implements Provider<IService> {
   constructor(
@@ -18,7 +11,7 @@ export class BillingProvider implements Provider<IService> {
     private readonly sdkProvider?: IService,
   ) {}
 
-  getProvider() {
+  getProvider(): IService {
     if (this.sdkProvider && this.restProvider) {
       throw new HttpErrors.NotAcceptable();
     } else if (this.sdkProvider) {
@@ -30,75 +23,7 @@ export class BillingProvider implements Provider<IService> {
     }
   }
 
-  async createCustomer(customerDto: TCustomer): Promise<TCustomer> {
-    return this.getProvider().createCustomer(customerDto);
-  }
-
-  async getCustomers(customerId: string): Promise<TCustomer> {
-    return this.getProvider().getCustomers(customerId);
-  }
-
-  async updateCustomerById(
-    tenantId: string,
-    customerDto: Partial<TCustomer>,
-  ): Promise<void> {
-    return this.getProvider().updateCustomerById(tenantId, customerDto);
-  }
-
-  async deleteCustomer(customerId: string): Promise<void> {
-    return this.getProvider().deleteCustomer(customerId);
-  }
-
-  async createPaymentSource(
-    paymentDto: TPaymentSource,
-  ): Promise<TPaymentSource> {
-    return this.getProvider().createPaymentSource(paymentDto);
-  }
-
-  async applyPaymentSourceForInvoice(
-    invoiceId: string,
-    transaction: Transaction,
-  ): Promise<TInvoice> {
-    return this.getProvider().applyPaymentSourceForInvoice(
-      invoiceId,
-      transaction,
-    );
-  }
-
-  async retrievePaymentSource(
-    paymentSourceId: string,
-  ): Promise<TPaymentSource> {
-    return this.getProvider().retrievePaymentSource(paymentSourceId);
-  }
-
-  async deletePaymentSource(paymentSourceId: string): Promise<void> {
-    return this.getProvider().deletePaymentSource(paymentSourceId);
-  }
-
-  async createInvoice(invoice: TInvoice): Promise<TInvoice> {
-    return this.getProvider().createInvoice(invoice);
-  }
-
-  async retrieveInvoice(invoiceId: string): Promise<TInvoice> {
-    return this.getProvider().retrieveInvoice(invoiceId);
-  }
-
-  async updateInvoice(
-    invoiceId: string,
-    invoice: Partial<TInvoice>,
-  ): Promise<TInvoice> {
-    return this.getProvider().updateInvoice(invoiceId, invoice);
-  }
-
-  async deleteInvoice(invoiceId: string): Promise<void> {
-    return this.getProvider().deleteInvoice(invoiceId);
-  }
-
-  async getPaymentStatus(invoiceId: string): Promise<boolean> {
-    return this.getProvider().getPaymentStatus(invoiceId);
-  }
-
-  value() {
+  value(): IService {
     return this.getProvider();
   }
 }

src/providers/sdk/chargebee/adapter/subscription.adapter.ts

@@ -5,6 +5,20 @@ import {
   TSubscriptionResult,
 } from '../../../../types';
 
+/**
+ * Local interface covering the Chargebee Subscription fields we map.
+ * Chargebee does not export a typed subscription object from its SDK,
+ * so we define the shape ourselves — no `any` needed.
+ */
+export interface RawChargebeeSubscription {
+  id: string;
+  status: string;
+  customer_id: string;
+  current_term_start?: number;
+  current_term_end?: number;
+  cancel_at_period_end?: boolean;
+}
+
 /**
  * Adapter that converts between the raw Chargebee Subscription object and the
  * provider-agnostic {@link TSubscriptionResult} shape used throughout the
@@ -17,10 +31,9 @@ import {
  * @example
  * ```ts
  * class MyAdapter extends ChargebeeSubscriptionAdapter {
- *   adaptToModel(resp: unknown): TSubscriptionResult {
+ *   adaptToModel(resp: RawChargebeeSubscription & {trial_end?: number}): TSubscriptionResult {
  *     const base = super.adaptToModel(resp);
- *     const raw = resp as {trial_end?: number};
- *     return {...base, trialEnd: raw.trial_end};
+ *     return {...base, trialEnd: resp.trial_end};
  *   }
  * }
  * // then:
@@ -36,9 +49,7 @@ export class ChargebeeSubscriptionAdapter
    *
    * @param resp - Raw Chargebee Subscription returned by the SDK.
    */
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  adaptToModel(resp: any): TSubscriptionResult {
-    // NOSONAR
+  adaptToModel(resp: RawChargebeeSubscription): TSubscriptionResult {
     return {
       id: resp.id,
       status: resp.status,

src/providers/sdk/chargebee/charge-bee.service.ts

@@ -1,8 +1,8 @@
 /* eslint-disable @typescript-eslint/naming-convention */
+import {randomUUID} from 'crypto';
 import {inject} from '@loopback/core';
 import chargebee from 'chargebee';
 import {
-  CollectionMethod,
   RecurringInterval,
   TInvoicePrice,
   TPrice,
@@ -353,8 +353,11 @@ export class ChargeBeeService implements IChargeBeeService {
   async createPrice(price: TPrice): Promise<TPrice> {
     try {
       // Chargebee requires an explicit ItemPrice ID or auto-generates one.
+      // randomUUID() is cryptographically secure (Node.js built-in v14.17+);
+      // the first 8-hex segment keeps the ID short and Chargebee-friendly.
       const priceId =
-        price.id ?? `${price.product}-${price.currency}-${Date.now()}`;
+        price.id ??
+        `${price.product}-${price.currency}-${randomUUID().split('-')[0]}`;
 
       const result = await chargebee.item_price
         .create({
@@ -410,24 +413,10 @@ export class ChargeBeeService implements IChargeBeeService {
    */
   async createSubscription(subscription: TSubscriptionCreate): Promise<string> {
     try {
+      const params =
+        this.chargebeeSubscriptionAdapter.adaptFromModel(subscription);
       const result = await chargebee.subscription
-        .create_with_items(subscription.customerId, {
-          subscription_items: [
-            {
-              item_price_id: subscription.priceRefId,
-            },
-          ],
-          discounts: [], // Required by Chargebee SDK type
-          ...(subscription.collectionMethod === CollectionMethod.SEND_INVOICE
-            ? {
-                auto_collection: 'off' as const,
-                // Only include net_term_days if explicitly provided and site has payment terms configured
-                ...(subscription.daysUntilDue !== undefined
-                  ? {net_term_days: subscription.daysUntilDue}
-                  : {}),
-              }
-            : {auto_collection: 'on' as const}),
-        })
+        .create_with_items(subscription.customerId, params)
         .request();
       return result.subscription.id;
     } catch (error) {
@@ -579,11 +568,7 @@ export class ChargeBeeService implements IChargeBeeService {
       // Using collect_payment without a payment_source triggers Chargebee to
       // send the hosted payment page link to the customer by email
       // (based on site notification settings).
-      await chargebee.invoice
-        .collect_payment(invoiceId, {
-          payment_source_id: undefined,
-        })
-        .request();
+      await chargebee.invoice.collect_payment(invoiceId, {}).request();
     } catch (error) {
       throw new Error(JSON.stringify(error));
     }
@@ -600,15 +585,14 @@ export class ChargeBeeService implements IChargeBeeService {
       const result = await chargebee.item.retrieve(productId).request();
       return result.item.status === 'active';
     } catch (error) {
-      // Chargebee throws a JSON error string with api_error_code for not-found
-      const message = JSON.stringify(error);
+      const cbError = error as {api_error_code?: string; http_status?: number};
       if (
-        message.includes('resource_not_found') ||
-        message.includes('not_found')
+        cbError.api_error_code === 'resource_not_found' ||
+        cbError.http_status === 404
       ) {
         return false;
       }
-      throw new Error(message);
+      throw new Error(JSON.stringify(error));
     }
   }
 }

src/providers/sdk/stripe/adapter/subscription.adapter.ts

@@ -33,17 +33,19 @@ export class StripeSubscriptionAdapter
    *
    * @param resp - Raw Stripe Subscription returned by the SDK.
    */
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  adaptToModel(resp: any): TSubscriptionResult {
-    const sub = resp as Stripe.Subscription;
+  adaptToModel(resp: Stripe.Subscription): TSubscriptionResult {
     return {
-      id: sub.id,
-      status: sub.status,
+      id: resp.id,
+      status: resp.status,
       customerId:
-        typeof sub.customer === 'string' ? sub.customer : sub.customer?.id,
-      currentPeriodStart: sub.current_period_start,
-      currentPeriodEnd: sub.current_period_end,
-      cancelAtPeriodEnd: sub.cancel_at_period_end,
+        typeof resp.customer === 'string'
+          ? resp.customer
+          : resp.customer && 'id' in resp.customer
+            ? resp.customer.id
+            : undefined,
+      currentPeriodStart: resp.current_period_start,
+      currentPeriodEnd: resp.current_period_end,
+      cancelAtPeriodEnd: resp.cancel_at_period_end,
     };
   }
 

src/providers/sdk/stripe/stripe.service.ts

@@ -314,11 +314,9 @@ export class StripeService implements IStripeService {
    * @returns The Stripe subscription ID.
    */
   async createSubscription(subscription: TSubscriptionCreate): Promise<string> {
+    const params = this.stripeSubscriptionAdapter.adaptFromModel(subscription);
     const created = await this.stripe.subscriptions.create({
-      customer: subscription.customerId,
-      items: [{price: subscription.priceRefId}],
-      collection_method: subscription.collectionMethod,
-      days_until_due: subscription.daysUntilDue,
+      ...params,
       payment_behavior: (this.stripeConfig.defaultPaymentBehavior ??
         'default_incomplete') as Stripe.SubscriptionCreateParams.PaymentBehavior,
     });
@@ -364,7 +362,9 @@ export class StripeService implements IStripeService {
       const newId = await this.createSubscription({
         customerId: existing.customer as string,
         priceRefId: updates.priceRefId ?? '',
-        collectionMethod: CollectionMethod.CHARGE_AUTOMATICALLY,
+        collectionMethod:
+          updates.collectionMethod ?? CollectionMethod.CHARGE_AUTOMATICALLY,
+        daysUntilDue: updates.daysUntilDue,
       });
       return {
         id: newId,
@@ -413,10 +413,13 @@ export class StripeService implements IStripeService {
         }),
       );
     } catch (err) {
-      // Non-fatal — subscription is already cancelled in Stripe; log for observability
-      console.info(
-        '[StripeService] cancelSubscription: invoice cleanup failed',
-        err,
+      // Invoice cleanup is best-effort after cancellation.
+      // Surface as a structured error so callers and APM tools can observe it.
+      throw Object.assign(
+        new Error(
+          `[StripeService] cancelSubscription: invoice cleanup failed for ${subscriptionId}`,
+        ),
+        {cause: err},
       );
     }
   }
@@ -440,8 +443,12 @@ export class StripeService implements IStripeService {
    */
   async resumeSubscription(subscriptionId: string): Promise<void> {
     await this.stripe.subscriptions.update(subscriptionId, {
-      /* eslint-disable-next-line @typescript-eslint/no-explicit-any */
-      pause_collection: '' as any, // NOSONAR – Stripe uses empty string to clear pause_collection
+      // Stripe clears pause_collection by passing an empty string.
+      // The SDK types do not model this; cast through unknown to preserve
+      // intent without using any.
+      // Ref: https://stripe.com/docs/billing/subscriptions/pause-payment
+      pause_collection:
+        '' as unknown as Stripe.SubscriptionUpdateParams.PauseCollection,
     });
   }
 

src/types.ts

@@ -195,6 +195,10 @@ export interface TSubscriptionUpdate {
   /** New price / plan reference ID. */
   priceRefId?: string;
   prorationBehavior?: ProrationBehavior;
+  /** Billing collection method to use when re-creating an incomplete subscription. */
+  collectionMethod?: CollectionMethod;
+  /** Number of days until the invoice is due (applicable for send_invoice). */
+  daysUntilDue?: number;
 }
 
 /**
@@ -203,7 +207,8 @@ export interface TSubscriptionUpdate {
 export interface TSubscriptionResult {
   id: string;
   status: string;
-  customerId: string;
+  /** Optional — customer may be deleted or unexpanded by the provider. */
+  customerId?: string;
   currentPeriodStart?: number;
   currentPeriodEnd?: number;
   cancelAtPeriodEnd?: boolean;
@@ -257,7 +262,9 @@ export interface ISubscriptionService {
   ): Promise<TSubscriptionResult>;
 
   /**
-   * Cancels a subscription immediately with proration and voids open invoices.
+   * Cancels a subscription. Providers may apply proration, credit notes, or
+   * invoice voiding automatically based on their own billing rules.
+   * After this call the subscription will no longer renew.
    */
   cancelSubscription(subscriptionId: string): Promise<void>;