refactor: remove redundant logging

supergoodsystems · Dec 11, 2023 · 0a883a2 · 0a883a2
1 parent e4cc41e
commit 0a883a2
Show file tree

Hide file tree

Showing 7 changed files with 9 additions and 120 deletions.
diff --git a/src/interceptor/http.request.ts b/src/interceptor/http.request.ts
@@ -8,16 +8,11 @@ import {
   normalizeClientRequestArgs,
   ClientRequestArgs
 } from './utils/request-args';
-import { pinoLogger } from '../logger';
-
-const logger = pinoLogger.child({ module: 'http request' });
 
 export function request(protocol: Protocol, options: NodeClientOptions) {
   return function interceptorsHttpRequest(
     ...args: ClientRequestArgs
   ): ClientRequest {
-    logger.debug('request call (protocol "%s"):', protocol, args);
-
     const clientRequestArgs = normalizeClientRequestArgs(
       `${protocol}:`,
       ...args

diff --git a/src/interceptor/utils/cloneObject.ts b/src/interceptor/utils/cloneObject.ts
@@ -1,28 +1,16 @@
-import { pinoLogger } from '../../logger';
-
-const logger = pinoLogger.child({ module: 'cloneObject' });
-
 function isPlainObject(obj?: Record<string, any>): boolean {
-  logger.debug('is plain object?', obj);
-
   if (obj == null || !obj.constructor?.name) {
-    logger.debug('given object is undefined, not a plain object...');
     return false;
   }
 
-  logger.debug('checking the object constructor:', obj.constructor.name);
   return obj.constructor.name === 'Object';
 }
 
 export function cloneObject<ObjectType extends Record<string, any>>(
   obj: ObjectType
 ): ObjectType {
-  logger.debug('cloning object:', obj);
-
   const enumerableProperties = Object.entries(obj).reduce<Record<string, any>>(
     (acc, [key, value]) => {
-      logger.debug('analyzing key-value pair:', key, value);
-
       // Recursively clone only plain objects, omitting class instances.
       acc[key] = isPlainObject(value) ? cloneObject(value) : value;
       return acc;

diff --git a/src/interceptor/utils/getIncomingMessageBody.ts b/src/interceptor/utils/getIncomingMessageBody.ts
@@ -3,16 +3,10 @@ import { IncomingMessage, IncomingHttpHeaders } from 'http';
 import { PassThrough } from 'stream';
 import * as zlib from 'zlib';
 
-import { pinoLogger } from '../../logger';
-
-const logger = pinoLogger.child({ module: 'http getIncomingMessageBody' });
-
 export function getIncomingMessageBody(
   response: IncomingMessage
 ): Promise<string> {
   return new Promise((resolve, reject) => {
-    logger.debug('cloning the original response...');
-
     // Pipe the original response to support non-clone
     // "response" input. No need to clone the response,
     // as we always have access to the full "response" input,
@@ -25,22 +19,18 @@ export function getIncomingMessageBody(
 
     const encoding = response.readableEncoding || 'utf8';
     stream.setEncoding(encoding);
-    logger.debug('using encoding:', encoding);
 
     let body = '';
 
     stream.on('data', (responseBody) => {
-      logger.debug('response body read:', responseBody);
       body += responseBody;
     });
 
     stream.once('end', () => {
-      logger.debug('response body end');
       resolve(body);
     });
 
     stream.once('error', (error) => {
-      logger.debug('error while reading response body:', error);
       reject(error);
     });
   });

diff --git a/src/interceptor/utils/getUrlByRequestOptions.ts b/src/interceptor/utils/getUrlByRequestOptions.ts
@@ -1,8 +1,5 @@
 import { Agent } from 'http';
 import { RequestOptions, Agent as HttpsAgent } from 'https';
-import { pinoLogger } from '../../logger';
-
-const logger = pinoLogger.child({ module: 'utils getUrlByRequestOptions' });
 
 // Request instance constructed by the "request" library
 // has a "self" property that has a "uri" field. This is
@@ -131,46 +128,20 @@ function getHostname(host: string, port?: number): string {
  * Creates a `URL` instance from a given `RequestOptions` object.
  */
 export function getUrlByRequestOptions(options: ResolvedRequestOptions): URL {
-  logger.debug('request options', options);
-
   if (options.uri) {
-    logger.debug(
-      'constructing url from explicitly provided "options.uri": %s',
-      options.uri
-    );
     return new URL(options.uri.href);
   }
 
-  logger.debug('figuring out url from request options...');
-
   const protocol = getProtocolByRequestOptions(options);
-  logger.debug('protocol', protocol);
-
   const host = getHostByRequestOptions(options);
-  logger.debug('host', host);
-
   const port = getPortByRequestOptions(options);
-  logger.debug('port', port);
-
   const hostname = getHostname(host, port);
-  logger.debug('hostname', hostname);
-
   const path = options.path || DEFAULT_PATH;
-  logger.debug('path', path);
-
   const credentials = getAuthByRequestOptions(options);
-  logger.debug('credentials', credentials);
-
-  const authString = credentials
-    ? `${credentials.username}:${credentials.password}@`
-    : '';
-  logger.debug('auth string:', authString);
-
   const url = new URL(`${protocol}//${hostname}${path}`);
+
   url.username = credentials?.username || '';
   url.password = credentials?.password || '';
 
-  logger.debug('created url:', url);
-
   return url;
 }
diff --git a/src/interceptor/utils/normalizeClientRequestWriteArgs.ts b/src/interceptor/utils/normalizeClientRequestWriteArgs.ts
@@ -1,7 +1,3 @@
-import { pinoLogger } from '../../logger';
-
-const logger = pinoLogger.child({ module: 'http normalizeWriteArgs' });
-
 export type ClientRequestWriteCallback = (error?: Error | null) => void;
 export type ClientRequestWriteArgs = [
   chunk: string | Buffer,
@@ -18,8 +14,6 @@ export type NormalizedClientRequestWriteArgs = [
 export function normalizeClientRequestWriteArgs(
   args: ClientRequestWriteArgs
 ): NormalizedClientRequestWriteArgs {
-  logger.debug('normalizing ClientRequest.write arguments...', args);
-
   const chunk = args[0];
   const encoding =
     typeof args[1] === 'string' ? (args[1] as BufferEncoding) : undefined;
@@ -30,10 +24,6 @@ export function normalizeClientRequestWriteArgs(
     encoding,
     callback
   ];
-  logger.debug(
-    'successfully normalized ClientRequest.write arguments:',
-    writeArgs
-  );
 
   return writeArgs;
 }
diff --git a/src/interceptor/utils/request-args.ts b/src/interceptor/utils/request-args.ts
@@ -16,9 +16,6 @@ import {
 } from './getUrlByRequestOptions';
 import { cloneObject } from './cloneObject';
 import { isObject } from './isObject';
-import { pinoLogger } from '../../logger';
-
-const logger = pinoLogger.child({ module: 'request-args' });
 
 export type HttpRequestCallback = (response: IncomingMessage) => void;
 
@@ -36,32 +33,25 @@ function resolveRequestOptions(
   // Calling `fetch` provides only URL to `ClientRequest`
   // without any `RequestOptions` or callback.
   if (typeof args[1] === 'undefined' || typeof args[1] === 'function') {
-    logger.debug('request options not provided, deriving from the url', url);
     return getRequestOptionsByUrl(url);
   }
 
   if (args[1]) {
-    logger.debug('has custom RequestOptions!', args[1]);
     const requestOptionsFromUrl = getRequestOptionsByUrl(url);
 
-    logger.debug('derived RequestOptions from the URL:', requestOptionsFromUrl);
-
     /**
      * Clone the request options to lock their state
      * at the moment they are provided to `ClientRequest`.
      * @see https://github.com/mswjs/interceptors/issues/86
      */
-    logger.debug('cloning RequestOptions...');
     const clonedRequestOptions = cloneObject(args[1]);
-    logger.debug('successfully cloned RequestOptions!', clonedRequestOptions);
 
     return {
       ...requestOptionsFromUrl,
       ...clonedRequestOptions
     };
   }
 
-  logger.debug('using an empty object as request options');
   return {} as RequestOptions;
 }
 
@@ -108,9 +98,6 @@ export function normalizeClientRequestArgs(
   let options: ResolvedRequestOptions;
   let callback: HttpRequestCallback | undefined;
 
-  logger.debug('arguments', args);
-  logger.debug('using default protocol:', defaultProtocol);
-
   // Support "http.request()" calls without any arguments.
   // That call results in a "GET http://localhost" request.
   if (args.length === 0) {
@@ -122,25 +109,14 @@ export function normalizeClientRequestArgs(
   // Convert a url string into a URL instance
   // and derive request options from it.
   if (typeof args[0] === 'string') {
-    logger.debug('first argument is a location string:', args[0]);
-
     url = new URL(args[0]);
-    logger.debug('created a url:', url);
-
-    const requestOptionsFromUrl = getRequestOptionsByUrl(url);
-    logger.debug('request options from url:', requestOptionsFromUrl);
-
     options = resolveRequestOptions(args, url);
-    logger.debug('resolved request options:', options);
-
     callback = resolveCallback(args);
   }
   // Handle a given URL instance as-is
   // and derive request options from it.
   else if (args[0] instanceof URL) {
     url = args[0];
-    logger.debug('first argument is a URL:', url);
-
     // Check if the second provided argument is RequestOptions.
     // If it is, check if "options.path" was set and rewrite it
     // on the input URL.
@@ -151,16 +127,12 @@ export function normalizeClientRequestArgs(
     }
 
     options = resolveRequestOptions(args, url);
-    logger.debug('derived request options:', options);
-
     callback = resolveCallback(args);
   }
   // Handle a legacy URL instance and re-normalize from either a RequestOptions object
   // or a WHATWG URL.
   else if ('hash' in args[0] && !('method' in args[0])) {
     const [legacyUrl] = args;
-    logger.debug('first argument is a legacy URL:', legacyUrl);
-
     if (legacyUrl.hostname === null) {
       /**
        * We are dealing with a relative url, so use the path as an "option" and
@@ -169,8 +141,6 @@ export function normalizeClientRequestArgs(
        * with the behaviour in ClientRequest.
        * @see https://github.com/nodejs/node/blob/d84f1312915fe45fe0febe888db692c74894c382/lib/_http_client.js#L122
        */
-      logger.debug('given legacy URL is relative (no hostname)');
-
       return isObject(args[1])
         ? normalizeClientRequestArgs(
             defaultProtocol,
@@ -183,9 +153,6 @@ export function normalizeClientRequestArgs(
             args[1] as HttpRequestCallback
           );
     }
-
-    logger.debug('given legacy url is absolute');
-
     // We are dealing with an absolute URL, so convert to WHATWG and try again.
     const resolvedUrl = new URL(legacyUrl.href);
 
@@ -204,15 +171,10 @@ export function normalizeClientRequestArgs(
   // and derive the URL instance from it.
   else if (isObject(args[0])) {
     options = args[0] as any;
-    logger.debug('first argument is RequestOptions:', options);
-
     // When handling a "RequestOptions" object without an explicit "protocol",
     // infer the protocol from the request issuing module (http/https).
     options.protocol = options.protocol || defaultProtocol;
-    logger.debug('normalized request options:', options);
-
     url = getUrlByRequestOptions(options);
-    logger.debug('created a URL from RequestOptions:', url.href);
 
     callback = resolveCallback(args);
   } else {
@@ -241,7 +203,6 @@ export function normalizeClientRequestArgs(
         : new HttpAgent();
 
     options.agent = agent;
-    logger.debug('resolved fallback agent:', agent);
   }
 
   /**
@@ -253,18 +214,9 @@ export function normalizeClientRequestArgs(
    * @see https://github.com/nodejs/node/blob/418ff70b810f0e7112d48baaa72932a56cfa213b/lib/_http_client.js#L157-L159
    */
   if (!options._defaultAgent) {
-    logger.debug(
-      'has no default agent, setting the default agent for "%s"',
-      options.protocol
-    );
-
     options._defaultAgent =
       options.protocol === 'https:' ? httpsGlobalAgent : httpGlobalAgent;
   }
 
-  logger.debug('successfully resolved url:', url.href);
-  logger.debug('successfully resolved options:', options);
-  logger.debug('successfully resolved callback:', callback);
-
   return [url, options, callback];
 }
diff --git a/test/e2e/core.e2e.test.ts b/test/e2e/core.e2e.test.ts
@@ -4,7 +4,7 @@ import axios from 'axios';
 import fetch from 'node-fetch';
 
 import Supergood from '../../src';
-import { errors } from '../../src/constants';
+import { LocalClientId, LocalClientSecret, errors } from '../../src/constants';
 import { EventRequestType } from '../../src/types';
 
 import { sleep } from '../../src/utils';
@@ -195,7 +195,10 @@ describe('core functionality', () => {
     test('ignores requests to ignored domains', async () => {
       await Supergood.init(
         {
-          config: { ignoredDomains: ['supergood-testbed.herokuapp.com'], allowLocalUrls: true },
+          config: {
+            ignoredDomains: ['supergood-testbed.herokuapp.com'],
+            allowLocalUrls: true
+          },
           clientId: SUPERGOOD_CLIENT_ID,
           clientSecret: SUPERGOOD_CLIENT_SECRET
         },
@@ -318,14 +321,14 @@ describe('core functionality', () => {
       await Supergood.init(
         {
           config: { ...SUPERGOOD_CONFIG, allowLocalUrls: true },
-          clientId: 'local-client-id',
-          clientSecret: 'local-client-secret'
+          clientId: LocalClientId,
+          clientSecret: LocalClientSecret
         },
         SUPERGOOD_SERVER
       );
       await axios.get(`${MOCK_DATA_SERVER}/posts`);
-      expect(postEventsMock).not.toHaveBeenCalled();
       await Supergood.close();
+      expect(postEventsMock).not.toHaveBeenCalled();
     });
   });
 });