Pending cleanups and fixes (#3526)

* Use logger instead of console * Replace with logger * Hide internal errors in SAML Fed request path * Replace with logger
boxyhq · Jan 28, 2025 · 497200b · 497200b
1 parent bd80d61
commit 497200b
Show file tree

Hide file tree

Showing 9 changed files with 27 additions and 16 deletions.
diff --git a/lib/api/default.ts b/lib/api/default.ts
@@ -1,3 +1,4 @@
+import { logger } from '@lib/logger';
 import { ApiError } from '../error';
 import type { NextApiRequest, NextApiResponse } from 'next';
 
@@ -25,7 +26,7 @@ export const defaultHandler = async (req: NextApiRequest, res: NextApiResponse,
     const message = err.message || 'Internal Server Error';
     const status = err.statusCode || 500;
 
-    console.error(`${req.method} ${req.url} - ${status} - ${message}`);
+    logger.error(`${req.method} ${req.url} - ${status} - ${message}`);
 
     res.status(status).json({ error: { message } });
   }

diff --git a/npm/src/db/sql/sql.ts b/npm/src/db/sql/sql.ts
@@ -106,7 +106,7 @@ class Sql implements DatabaseDriver {
         }
         break;
       } catch (err) {
-        console.error(
+        this.logger.error(
           `error in index namespace execution for engine: ${this.options.engine}, type: ${sqlType} err: ${err}`
         );
         await dbutils.sleep(1000);
@@ -144,7 +144,7 @@ class Sql implements DatabaseDriver {
 
       this.timerId = setTimeout(this.ttlCleanup, this.options.ttl! * 1000);
     } else {
-      console.warn(
+      this.logger.warn(
         `Warning: ttl cleanup not enabled in ${sqlType} with engine ${this.options.engine}, set both "ttl" and "cleanupLimit" options to enable it!`
       );
     }
@@ -176,7 +176,7 @@ class Sql implements DatabaseDriver {
         }
       }
     } catch (err) {
-      console.error('Error running indexNamespace:', err);
+      this.logger.error('Error running indexNamespace:', err);
     }
   }
 

diff --git a/npm/src/ee/identity-federation/app.ts b/npm/src/ee/identity-federation/app.ts
@@ -10,7 +10,7 @@ import type {
   AppRequestParams,
   Index,
 } from '../../typings';
-import { fedAppID, clientIDFederatedPrefix } from '../../controller/utils';
+import { fedAppID, clientIDFederatedPrefix, GENERIC_ERR_STRING } from '../../controller/utils';
 import { JacksonError } from '../../controller/error';
 import { getDefaultCertificate } from '../../saml/x509';
 import { IndexNames, validateTenantAndProduct } from '../../controller/utils';
@@ -377,7 +377,11 @@ export class App {
     await throwIfInvalidLicense(this.opts.boxyhqLicenseKey);
 
     if (!entityId) {
-      throw new JacksonError('Missing required parameters. Required parameters are: entityId', 400);
+      throw new JacksonError(
+        GENERIC_ERR_STRING,
+        400,
+        'Missing required parameters. Required parameters are: entityId'
+      );
     }
 
     const apps: IdentityFederationApp[] = (
@@ -388,7 +392,7 @@ export class App {
     ).data;
 
     if (!apps || apps.length === 0) {
-      throw new JacksonError('Identity Federation app not found', 404);
+      throw new JacksonError(GENERIC_ERR_STRING, 404, 'Identity Federation app not found');
     }
 
     return apps[0];

diff --git a/npm/src/ee/product/index.ts b/npm/src/ee/product/index.ts
@@ -1,12 +1,12 @@
 import { JacksonError } from '../../controller/error';
 import { throwIfInvalidLicense } from '../common/checkLicense';
-import type { Storable, JacksonOption, ProductConfig } from '../../typings';
+import type { Storable, ProductConfig, JacksonOptionWithRequiredLogger } from '../../typings';
 
 export class ProductController {
   private productStore: Storable;
-  private opts: JacksonOption;
+  private opts: JacksonOptionWithRequiredLogger;
 
-  constructor({ productStore, opts }: { productStore: Storable; opts: JacksonOption }) {
+  constructor({ productStore, opts }: { productStore: Storable; opts: JacksonOptionWithRequiredLogger }) {
     this.productStore = productStore;
     this.opts = opts;
   }
@@ -17,7 +17,7 @@ export class ProductController {
     const productConfig = (await this.productStore.get(productId)) as ProductConfig;
 
     // if (!productConfig) {
-    //   console.error(`Product config not found for ${productId}`);
+    //   this.opts.logger.error(`Product config not found for ${productId}`);
     // }
 
     return {

diff --git a/npm/src/index.ts b/npm/src/index.ts
@@ -110,7 +110,10 @@ export const controllers = async (
 
   const ssoTraces = new SSOTraces({ tracesStore, opts });
   const eventController = new EventController({ opts: opts as JacksonOptionWithRequiredLogger });
-  const productController = new ProductController({ productStore, opts });
+  const productController = new ProductController({
+    productStore,
+    opts: opts as JacksonOptionWithRequiredLogger,
+  });
 
   const connectionAPIController = new ConnectionAPIController({
     connectionStore,

diff --git a/pages/api/health.ts b/pages/api/health.ts
@@ -2,6 +2,7 @@ import { NextApiRequest, NextApiResponse } from 'next';
 
 import jackson from '@lib/jackson';
 import packageInfo from '../../package.json';
+import { logger } from '@lib/logger';
 
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
   try {
@@ -16,7 +17,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
       version: packageInfo.version,
     });
   } catch (err: any) {
-    console.error('HealthCheck failed', err);
+    logger.error(err, 'HealthCheck failed');
     const { statusCode = 503 } = err;
     res.status(statusCode).json({});
   }

diff --git a/pages/api/oauth/saml.ts b/pages/api/oauth/saml.ts
@@ -29,7 +29,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
 
     if (redirect_url) {
       if (error) {
-        console.error(`Error processing SAML IdP response: ${error}`);
+        logger.error(`Error processing SAML IdP response: ${error}`);
       }
       res.redirect(302, redirect_url);
       return;

diff --git a/pages/api/v1/saml/config.ts b/pages/api/v1/saml/config.ts
@@ -3,6 +3,7 @@
 import jackson from '@lib/jackson';
 import { NextApiRequest, NextApiResponse } from 'next';
 import type { DelConnectionsQuery, GetConfigQuery } from '@boxyhq/saml-jackson';
+import { logger } from '@lib/logger';
 
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
   try {
@@ -19,7 +20,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
       throw { message: 'Method not allowed', statusCode: 405 };
     }
   } catch (err: any) {
-    console.error('config api error:', err);
+    logger.error(err, 'config api error');
     const { message, statusCode = 500 } = err;
 
     res.status(statusCode).send(message);

diff --git a/pages/api/v1/saml/config/exists.ts b/pages/api/v1/saml/config/exists.ts
@@ -3,6 +3,7 @@
 import jackson from '@lib/jackson';
 import { NextApiRequest, NextApiResponse } from 'next';
 import type { GetConfigQuery } from '@boxyhq/saml-jackson';
+import { logger } from '@lib/logger';
 
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
   try {
@@ -18,7 +19,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
       throw { message: 'Method not allowed', statusCode: 405 };
     }
   } catch (err: any) {
-    console.error('config api error:', err);
+    logger.error(err, 'config api error');
     const { message, statusCode = 500 } = err;
 
     res.status(statusCode).send(message);