chore(logger): Remove debugging and do a little housekeeping

pact-foundation · Jan 29, 2021 · 6cded49 · 6cded49
1 parent a3e67ef
commit 6cded49
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 67 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -53,7 +53,6 @@
     "@types/pino-std-serializers": "^2.4.1",
     "@types/q": "1.0.7",
     "@types/request": "2.48.2",
-    "@types/sonic-boom": "^0.7.0",
     "chalk": "2.3.1",
     "check-types": "7.3.0",
     "cross-spawn": "^7.0.1",
@@ -69,7 +68,6 @@
     "quick-format-unescaped": "^4.0.1",
     "request": "2.88.0",
     "rimraf": "2.6.2",
-    "sonic-boom": "^1.1.0",
     "sumchecker": "^2.0.2",
     "tar": "4.4.2",
     "underscore": "1.8.3",
@@ -89,6 +87,7 @@
     "@types/mkdirp": "^0.5.2",
     "@types/mocha": "2.2.48",
     "@types/node": "9.4.6",
+    "@types/pino": "^6.3.5",
     "@types/rimraf": "^2.0.2",
     "@types/tar": "^4.0.3",
     "@types/underscore": "1.8.7",

diff --git a/src/logger.ts b/src/logger.ts
@@ -1,30 +1,25 @@
 import pino = require('pino');
-import SonicBoom = require('sonic-boom');
 
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const pkg = require('../package.json');
 
-let level = (process.env.LOGLEVEL || 'info').toLowerCase();
-
 export type Logger = pino.Logger;
-export type LogDest = SonicBoom;
+export type LogLevels = pino.Level;
 
-export const create = (destination?: pino.DestinationStream): Logger =>
-	pino(
-		{
-			level,
-			prettyPrint: {
-				messageFormat: `pact@${pkg.version} -- {msg}`,
-				translateTime: true,
-			},
+const DEFAULT_LEVEL: LogLevels = (process.env.LOGLEVEL || 'info') as LogLevels;
+
+const createLogger = (level: LogLevels = DEFAULT_LEVEL): Logger =>
+	pino({
+		level: level.toLowerCase(),
+		prettyPrint: {
+			messageFormat: `pact@${pkg.version}: {msg}`,
+			translateTime: true,
 		},
-		destination || pino.destination(1),
-	);
+	});
 
-let logger: pino.Logger = create();
+const logger: pino.Logger = createLogger();
 
 export const setLogLevel = (
-	logger: Logger,
 	wantedLevel?: pino.Level | number,
 ): number | void => {
 	if (wantedLevel) {
@@ -33,13 +28,7 @@ export const setLogLevel = (
 				? wantedLevel.toLowerCase()
 				: logger.levels.labels[wantedLevel];
 	}
-
 	return logger.levels.values[logger.level];
 };
 
-export const createDestination = (path: string): LogDest =>
-	pino.destination(path);
-
-export type LogLevels = pino.Level;
-
 export default logger;
diff --git a/src/pact.ts b/src/pact.ts
@@ -10,7 +10,7 @@ import canDeployFactory, {
 	CanDeployResponse,
 } from './can-deploy';
 import pactEnvironment from './pact-environment';
-import logger, { LogLevels, setLogLevel /* endDestination */ } from './logger';
+import logger, { LogLevels, setLogLevel } from './logger';
 import { AbstractService } from './service';
 import * as _ from 'underscore';
 import mkdirp = require('mkdirp');
@@ -43,8 +43,8 @@ export class Pact {
 		process.once('SIGINT', () => process.exit());
 	}
 
-	public logLevel(level?: LogLevels): number | void {
-		return setLogLevel(logger, level);
+	public logLevel(level?: LogLevels | number): number | void {
+		return setLogLevel(level);
 	}
 
 	// Creates server with specified options

diff --git a/src/server.ts b/src/server.ts
@@ -70,16 +70,9 @@ export class Server extends AbstractService {
 			}
 		}
 
-		let opts = options;
-
-		// Need to uppercase logLevel for ruby
-		if (options.logLevel) {
-			opts.logLevel = options.logLevel.toUpperCase() as LogLevel;
-		}
-
 		super(
 			pact.mockServicePath,
-			opts,
+			options,
 			{
 				port: '--port',
 				host: '--host',

diff --git a/src/service.ts b/src/service.ts
@@ -3,12 +3,7 @@ import fs = require('fs');
 import events = require('events');
 import http = require('request');
 import q = require('q');
-import defaultLogger, {
-	createDestination,
-	create as createLogger,
-	Logger,
-	LogDest,
-} from './logger';
+import logger /*, { setLogLevel }*/ from './logger';
 import spawn, { CliVerbOptions } from './spawn';
 import { ChildProcess } from 'child_process';
 import mkdirp = require('mkdirp');
@@ -46,9 +41,6 @@ export abstract class AbstractService extends events.EventEmitter {
 	protected __cliVerb?: CliVerbOptions;
 	protected __serviceCommand: string;
 
-	private __logger: Logger = defaultLogger;
-	private __logDest?: LogDest;
-
 	protected constructor(
 		command: string,
 		options: ServiceOptions,
@@ -58,14 +50,20 @@ export abstract class AbstractService extends events.EventEmitter {
 	) {
 		super();
 
-		if (options.log) {
-			this.__logDest = createDestination(
-				path.join(path.dirname(options.log), 'testy-loggy.log'),
-			);
+		// Set logger first
+		if (options.logLevel) {
+			//	setLogLevel(options.logLevel);
+			// Pact-node's logger supports fatal and trace,
+			// but the ruby binary doesn't. So we map those.
+			if ((options.logLevel as string) === 'fatal') {
+				options.logLevel = 'error';
+			} else if ((options.logLevel as string) === 'trace') {
+				options.logLevel = 'debug';
+			}
 
-			this.__logger = createLogger(this.__logDest);
+			// Then need to uppercase log level for ruby
+			options.logLevel = options.logLevel.toUpperCase() as LogLevel;
 		}
-
 		// defaults
 		options.ssl = options.ssl || false;
 		options.cors = options.cors || false;
@@ -79,7 +77,7 @@ export abstract class AbstractService extends events.EventEmitter {
 			checkTypes.assert.inRange(options.port, 0, 65535);
 
 			if (checkTypes.not.inRange(options.port, 1024, 49151)) {
-				this.__logger.warn(
+				logger.warn(
 					'Like a Boss, you used a port outside of the recommended range (1024 to 49151); I too like to live dangerously.',
 				);
 			}
@@ -150,7 +148,7 @@ export abstract class AbstractService extends events.EventEmitter {
 
 	public start(): q.Promise<AbstractService> {
 		if (this.__instance && this.__instance.connected) {
-			this.__logger.warn(
+			logger.warn(
 				`You already have a process running with PID: ${this.__instance.pid}`,
 			);
 			return q.resolve(this);
@@ -171,15 +169,15 @@ export abstract class AbstractService extends events.EventEmitter {
 						// read the port number from it
 						this.__instance.stdout.removeListener('data', catchPort);
 					}
-					this.__logger.info(`Pact running on port ${this.options.port}`);
+					logger.info(`Pact running on port ${this.options.port}`);
 				}
 			};
 
 			this.__instance.stdout.on('data', catchPort);
 		}
 
 		this.__instance.stderr.on('data', data =>
-			this.__logger.error(`Pact Binary Error: ${data}`),
+			logger.error(`Pact Binary Error: ${data}`),
 		);
 
 		// check service is available
@@ -210,22 +208,9 @@ export abstract class AbstractService extends events.EventEmitter {
 
 	// Deletes this instance and emit an event
 	public delete(): q.Promise<AbstractService> {
-		const dest = this.__logDest;
-
-		console.log('!!!!!!!!!!!!!!!!!!!!!!!!!', dest);
-
 		return this.stop().then(() => {
 			this.emit(AbstractService.Events.DELETE_EVENT, this);
 
-			if (dest) {
-				console.log('>>>>>>>>>>>>>>>>>>>>. has dest', dest);
-				dest.end();
-			} else {
-				console.log('>>>>>>>>>>>>>>>>>>>> no dest!!!!', dest);
-			}
-
-			this.__logger = defaultLogger;
-
 			return this;
 		});
 	}
@@ -342,6 +327,7 @@ export interface ServiceOptions {
 	logLevel?: LogLevel;
 }
 
+// This is the pact binary's log level, which is a subset of the log levels for pact-node
 export type LogLevel = 'debug' | 'info' | 'warn' | 'error';
 
 export interface HTTPConfig {