From b5c5296e7224924a70b12f643686af65d55e8fb5 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Thu, 17 Apr 2025 09:41:40 +0100
Subject: [PATCH] chore(server/logging): simplify withOperationLogging
 parameters - complex error handling should be done by the business operation,
 we do not need to pass in a callback

---
 .../server/modules/gatekeeper/rest/billing.ts | 77 ++++++++++---------
 .../observability/domain/businessLogging.ts   | 11 +--
 2 files changed, 42 insertions(+), 46 deletions(-)

diff --git a/packages/server/modules/gatekeeper/rest/billing.ts b/packages/server/modules/gatekeeper/rest/billing.ts
index 7b36d583b..fcc93cc80 100644
--- a/packages/server/modules/gatekeeper/rest/billing.ts
+++ b/packages/server/modules/gatekeeper/rest/billing.ts
@@ -29,10 +29,7 @@ import {
   OperationStatus,
   stripeEventId
 } from '@/observability/domain/fields'
-import {
-  logErrorThenThrow,
-  withOperationLogging
-} from '@/observability/domain/businessLogging'
+import { withOperationLogging } from '@/observability/domain/businessLogging'
 
 export const getBillingRouter = (): Router => {
   const router = Router()
@@ -121,44 +118,50 @@ export const getBillingRouter = (): Router => {
             logger.info(OperationStatus.start, '[{operationName} ({operationStatus})] ')
 
             await withOperationLogging(
-              async () =>
-                await withTransaction(
-                  async ({ db }) => {
-                    const completeCheckout = completeCheckoutSessionFactory({
-                      getCheckoutSession: getCheckoutSessionFactory({ db }),
-                      updateCheckoutSessionStatus: updateCheckoutSessionStatusFactory({
-                        db
-                      }),
-                      upsertPaidWorkspacePlan: upsertPaidWorkspacePlanFactory({ db }),
-                      upsertWorkspaceSubscription: upsertWorkspaceSubscriptionFactory({
-                        db
-                      }),
-                      getSubscriptionData: getSubscriptionDataFactory({
-                        stripe
-                      }),
-                      emitEvent: getEventBus().emit
-                    })
+              async () => {
+                try {
+                  await withTransaction(
+                    async ({ db }) => {
+                      const completeCheckout = completeCheckoutSessionFactory({
+                        getCheckoutSession: getCheckoutSessionFactory({ db }),
+                        updateCheckoutSessionStatus: updateCheckoutSessionStatusFactory(
+                          {
+                            db
+                          }
+                        ),
+                        upsertPaidWorkspacePlan: upsertPaidWorkspacePlanFactory({ db }),
+                        upsertWorkspaceSubscription: upsertWorkspaceSubscriptionFactory(
+                          {
+                            db
+                          }
+                        ),
+                        getSubscriptionData: getSubscriptionDataFactory({
+                          stripe
+                        }),
+                        emitEvent: getEventBus().emit
+                      })
 
-                    return completeCheckout({
-                      sessionId: session.id,
-                      subscriptionId
-                    })
-                  },
-                  { db }
-                ),
+                      return completeCheckout({
+                        sessionId: session.id,
+                        subscriptionId
+                      })
+                    },
+                    { db }
+                  )
+                } catch (e) {
+                  if (e instanceof WorkspaceAlreadyPaidError) {
+                    // ignore the request, this is prob a replay from stripe
+                    logger.info('Workspace is already paid, ignoring')
+                  } else {
+                    throw e
+                  }
+                }
+              },
               {
                 logger,
                 operationName: 'completeCheckoutSession',
                 operationDescription:
-                  'Payment succeeded or Stripe session completed, and payment was paid',
-                errorHandler: async (err, logger) => {
-                  if (err instanceof WorkspaceAlreadyPaidError) {
-                    // ignore the request, this is prob a replay from stripe
-                    logger.info('Workspace is already paid, ignoring')
-                  } else {
-                    logErrorThenThrow(err, logger)
-                  }
-                }
+                  'Payment succeeded or Stripe session completed, and payment was paid'
               }
             )
 
diff --git a/packages/server/observability/domain/businessLogging.ts b/packages/server/observability/domain/businessLogging.ts
index d5b08221d..76ae5e6cf 100644
--- a/packages/server/observability/domain/businessLogging.ts
+++ b/packages/server/observability/domain/businessLogging.ts
@@ -5,12 +5,6 @@ import {
   OperationStatus
 } from '@/observability/domain/fields'
 import { logWithErr } from '@/observability/utils/logLevels'
-import { MaybeAsync } from '@speckle/shared'
-
-export const logErrorThenThrow = (err: unknown, logger: Logger) => {
-  logWithErr(logger, err, OperationStatus.failure, OperationLogLinePrefix)
-  throw err
-}
 
 /**
  * @description withOperationLogging is intended to be used for adding observability to high-level 'business' operations
@@ -26,11 +20,9 @@ export const withOperationLogging = async <T>(
     logger: Logger
     operationName: string
     operationDescription?: string
-    errorHandler?: (err: unknown, logger: Logger) => MaybeAsync<T>
   }
 ): Promise<T> => {
   const { operationName, operationDescription } = params
-  const errorHandler = params.errorHandler || logErrorThenThrow
   const logger = params.logger.child(OperationName(operationName))
 
   try {
@@ -44,6 +36,7 @@ export const withOperationLogging = async <T>(
     logger.info(OperationStatus.success, OperationLogLinePrefix)
     return results
   } catch (err) {
-    return await errorHandler(err, logger)
+    logWithErr(logger, err, OperationStatus.failure, OperationLogLinePrefix)
+    throw err
   }
 }