From b14b44faa44a2f4c524ba390a4028d91010b79af Mon Sep 17 00:00:00 2001
From: Princi Vershwal <vershwal.princi@gmail.com>
Date: Tue, 10 Sep 2024 16:47:12 +0530
Subject: [PATCH] ENG-1526 Added logs and sentry messages in Router Controller
 (#20955)

Ref:
https://linear.app/tryghost/issue/ENG-1526/errors-from-members-api-routercontroller-are-being-lost

The try/catch/re-throw pattern, that hides errors, is used throughout
the RouterController.js file.
I have not changed the try/catch/re-throw pattern because it helps in
sending clean messages to the users. We may not want to return internal
errors as API responses.
I have added logs and Sentry messages that will help us debug without
losing error messages.
---
 .../lib/controllers/RouterController.js          | 16 +++++++++++++++-
 ghost/members-api/lib/members-api.js             |  3 ++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/ghost/members-api/lib/controllers/RouterController.js b/ghost/members-api/lib/controllers/RouterController.js
index 486022263a..78aa4a31f1 100644
--- a/ghost/members-api/lib/controllers/RouterController.js
+++ b/ghost/members-api/lib/controllers/RouterController.js
@@ -40,6 +40,7 @@ module.exports = class RouterController {
      * @param {any} deps.sendEmailWithMagicLink
      * @param {{isSet(name: string): boolean}} deps.labsService
      * @param {any} deps.newslettersService
+     * @param {any} deps.sentry
      */
     constructor({
         offersAPI,
@@ -54,7 +55,8 @@ module.exports = class RouterController {
         memberAttributionService,
         sendEmailWithMagicLink,
         labsService,
-        newslettersService
+        newslettersService,
+        sentry
     }) {
         this._offersAPI = offersAPI;
         this._paymentsService = paymentsService;
@@ -69,6 +71,7 @@ module.exports = class RouterController {
         this._memberAttributionService = memberAttributionService;
         this.labsService = labsService;
         this._newslettersService = newslettersService;
+        this._sentry = sentry || undefined;
     }
 
     async ensureStripe(_req, res, next) {
@@ -80,6 +83,7 @@ module.exports = class RouterController {
             await this._stripeAPIService.ready();
             next();
         } catch (err) {
+            logging.error(err);
             res.writeHead(500);
             return res.end('There was an error configuring stripe');
         }
@@ -102,6 +106,7 @@ module.exports = class RouterController {
                 email = claims && claims.sub;
             }
         } catch (err) {
+            logging.error(err);
             res.writeHead(401);
             return res.end('Unauthorized');
         }
@@ -270,6 +275,8 @@ module.exports = class RouterController {
                     throw undefined;
                 }
             } catch (err) {
+                logging.error(err);
+                this._sentry?.captureException?.(err);
                 throw new BadRequestError({
                     message: tpl(messages.tierNotFound),
                     context: 'Tier with id "' + tierId + '" not found'
@@ -351,6 +358,8 @@ module.exports = class RouterController {
 
             return {url: paymentLink};
         } catch (err) {
+            logging.error(err);
+            this._sentry?.captureException?.(err);
             throw new BadRequestError({
                 err,
                 message: tpl(messages.unableToCheckout)
@@ -381,6 +390,8 @@ module.exports = class RouterController {
 
             return {url: paymentLink};
         } catch (err) {
+            logging.error(err);
+            this._sentry?.captureException?.(err);
             throw new BadRequestError({
                 err,
                 message: tpl(messages.unableToCheckout)
@@ -418,6 +429,8 @@ module.exports = class RouterController {
                         isAuthenticated = true;
                     }
                 } catch (err) {
+                    logging.error(err);
+                    this._sentry?.captureException?.(err);
                     throw new UnauthorizedError({err});
                 }
             } else if (req.body.customerEmail) {
@@ -592,6 +605,7 @@ module.exports = class RouterController {
                 res.writeHead(400);
                 return res.end('Bad Request.');
             }
+            logging.error(err);
 
             // Let the normal error middleware handle this error
             throw err;
diff --git a/ghost/members-api/lib/members-api.js b/ghost/members-api/lib/members-api.js
index a726ad9eab..14fbad3aad 100644
--- a/ghost/members-api/lib/members-api.js
+++ b/ghost/members-api/lib/members-api.js
@@ -192,7 +192,8 @@ module.exports = function MembersAPI({
         sendEmailWithMagicLink,
         memberAttributionService,
         labsService,
-        newslettersService
+        newslettersService,
+        sentry
     });
 
     const wellKnownController = new WellKnownController({