Skip to content

Commit

Permalink
Remove logging func
Browse files Browse the repository at this point in the history
  • Loading branch information
Shane DeWael committed Mar 5, 2019
1 parent c640509 commit d67aecf
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 131 deletions.
3 changes: 0 additions & 3 deletions docs/_pages/rtm_client.md
Expand Up @@ -344,9 +344,6 @@ to an object that has the following methods:
| `error()` | `...msgs: any[]` | `void` |


**NOTE**: While the use of logging functions is deprecated, the `logger` option will still currently accept a function
whose method signature matches `fn(level: string, message: string)`

```javascript
const fs = require('fs');
const { RTMClient, LogLevel } = require('@slack/client');
Expand Down
4 changes: 0 additions & 4 deletions docs/_pages/web_client.md
Expand Up @@ -420,10 +420,6 @@ to an object that has the following methods:
| `warn()` | `...msgs: any[]` | `void` |
| `error()` | `...msgs: any[]` | `void` |


**NOTE**: While the use of logging functions is deprecated, the `logger` option will still currently accept a function
whose method signature matches `fn(level: string, message: string)`.

```javascript
const fs = require('fs');
const { WebClient, LogLevel } = require('@slack/client');
Expand Down
12 changes: 3 additions & 9 deletions src/KeepAlive.ts
@@ -1,12 +1,11 @@
import { RTMClient, ErrorCode } from './';
import EventEmitter = require('eventemitter3'); // tslint:disable-line:import-name no-require-imports
import { LogLevel, Logger, LoggingFunc, getLogger, loggerFromLoggingFunc, isLoggingFunc } from './logger';
import { LogLevel, Logger, getLogger } from './logger';
import { errorWithCode } from './errors';
const pkg = require('../package.json'); // tslint:disable-line:no-require-imports no-var-requires

export interface KeepAliveOptions {
/** Custom logger. Using a LoggingFunc is deprecated. */
logger?: Logger | LoggingFunc;
logger?: Logger;
logLevel?: LogLevel;
/** How long (in ms) to wait before sending a ping message to keep the connection alive */
clientPingTimeout?: number;
Expand Down Expand Up @@ -95,12 +94,7 @@ export class KeepAlive extends EventEmitter {
this.recommendReconnect = false;

// Logging
if (logger !== undefined && isLoggingFunc(logger)) {
this.logger = loggerFromLoggingFunc(KeepAlive.loggerName, logger, logLevel);
this.logger.warn('Using a logging function is deprecated. Use a Logger object instead.');
} else {
this.logger = getLogger(KeepAlive.loggerName, logLevel, logger);
}
this.logger = getLogger(KeepAlive.loggerName, logLevel, logger);
}

/**
Expand Down
12 changes: 3 additions & 9 deletions src/RTMClient.ts
Expand Up @@ -5,7 +5,7 @@ import WebSocket = require('ws'); // tslint:disable-line:import-name no-require-
import Finity, { StateMachine } from 'finity'; // tslint:disable-line:import-name
import PQueue = require('p-queue'); // tslint:disable-line:import-name no-require-imports
import PCancelable = require('p-cancelable'); // tslint:disable-line:import-name no-require-imports
import { LogLevel, Logger, LoggingFunc, getLogger, loggerFromLoggingFunc, isLoggingFunc } from './logger';
import { LogLevel, Logger, getLogger } from './logger';
import { RetryOptions } from './retry-policies';
import { KeepAlive } from './KeepAlive';
import { WebClient, WebAPICallResult, WebAPICallError, ErrorCode, CodedError } from './';
Expand Down Expand Up @@ -345,12 +345,7 @@ export class RTMClient extends EventEmitter {
}, this);

// Logging
if (logger !== undefined && isLoggingFunc(logger)) {
this.logger = loggerFromLoggingFunc(RTMClient.loggerName, logger, logLevel);
this.logger.warn('Using a logging function is deprecated. Use a Logger object instead.');
} else {
this.logger = getLogger(RTMClient.loggerName, logLevel, logger);
}
this.logger = getLogger(RTMClient.loggerName, logLevel, logger);

this.stateMachine = Finity.start(this.stateMachineConfig);

Expand Down Expand Up @@ -658,8 +653,7 @@ export default RTMClient;
// NOTE: add an experimental flag to turn off KeepAlive
export interface RTMClientOptions {
slackApiUrl?: string;
/** Custom logger. Using a LoggingFunc is deprecated. */
logger?: Logger | LoggingFunc;
logger?: Logger;
logLevel?: LogLevel;
retryConfig?: RetryOptions;
agent?: Agent;
Expand Down
18 changes: 0 additions & 18 deletions src/WebClient.spec.js
Expand Up @@ -71,15 +71,6 @@ describe('WebClient', function () {
const capturedOutput = this.capture.getCapturedText();
assert.isEmpty(capturedOutput);
});
it('sends logs to a logger function and not to stdout', function () {
const stub = sinon.stub();
const debuggingClient = new WebClient(token, { logLevel: LogLevel.DEBUG, logger: stub });
assert.isTrue(stub.called);
const call = stub.lastCall;
assert.isTrue(call.calledWithMatch('debug'));
const capturedOutput = this.capture.getCapturedText();
assert.isEmpty(capturedOutput);
});
afterEach(function () {
this.capture.stopCapture();
});
Expand Down Expand Up @@ -1144,15 +1135,6 @@ describe('WebClient', function () {
this.capture.stopCapture();
done();
});
it('should warn when using a logging func', function (done) {
const stub = sinon.spy();
new WebClient(token, { logger: stub });
assert.isTrue(stub.called);
const call = stub.firstCall;
assert.isTrue(call.calledWithMatch('warn'));
this.capture.stopCapture();
done();
});
});

afterEach(function () {
Expand Down
12 changes: 3 additions & 9 deletions src/WebClient.ts
Expand Up @@ -14,7 +14,7 @@ import axios, { AxiosInstance, AxiosResponse } from 'axios';
import FormData = require('form-data'); // tslint:disable-line:no-require-imports import-name
import { awaitAndReduce, callbackify, getUserAgent, delay, AgentOption, TLSOptions, agentForScheme } from './util';
import { CodedError, errorWithCode, ErrorCode } from './errors';
import { LogLevel, Logger, LoggingFunc, getLogger, loggerFromLoggingFunc, isLoggingFunc } from './logger';
import { LogLevel, Logger, getLogger } from './logger';
import retryPolicies, { RetryOptions } from './retry-policies';
import Method, * as methods from './methods'; // tslint:disable-line:import-name

Expand Down Expand Up @@ -160,12 +160,7 @@ export class WebClient extends EventEmitter {
this.rejectRateLimitedCalls = rejectRateLimitedCalls;

// Logging
if (logger !== undefined && isLoggingFunc(logger)) {
this.logger = loggerFromLoggingFunc(WebClient.loggerName, logger, logLevel);
this.logger.warn('Using a logging function is deprecated. Use a Logger object instead.');
} else {
this.logger = getLogger(WebClient.loggerName, logLevel, logger);
}
this.logger = getLogger(WebClient.loggerName, logLevel, logger);

this.axios = axios.create({
baseURL: slackApiUrl,
Expand Down Expand Up @@ -882,8 +877,7 @@ export default WebClient;

export interface WebClientOptions {
slackApiUrl?: string;
/** Custom logger. Using a LoggingFunc is deprecated. */
logger?: Logger | LoggingFunc;
logger?: Logger;
logLevel?: LogLevel;
maxRequestConcurrency?: number;
retryConfig?: RetryOptions;
Expand Down
1 change: 0 additions & 1 deletion src/index.ts
@@ -1,6 +1,5 @@
export {
Logger,
LoggingFunc,
LogLevel,
} from './logger';

Expand Down
78 changes: 0 additions & 78 deletions src/logger.ts
Expand Up @@ -153,14 +153,6 @@ export class ConsoleLogger implements Logger {
}
}

/**
* Interface for functions where this package's logs can be re-routed
* @deprecated
*/
export interface LoggingFunc {
(level: LogLevel, message: string): void;
}

let instanceCount = 0;

/**
Expand All @@ -183,73 +175,3 @@ export function getLogger(name: string, level: LogLevel, existingLogger?: Logger

return logger;
}

/**
* INTERNAL function for transforming an external LoggingFunc type into the Logger interface.
*/
export function loggerFromLoggingFunc(name: string, loggingFunc: LoggingFunc, level: LogLevel): Logger {
// Get a unique ID for the logger.
const instanceId = instanceCount;
instanceCount += 1;

let loggerName = `${name}:${instanceId}`;
let loggerLevel = level;

// Set up the logger.
const logger: Logger = {
setLevel(level): void {
loggerLevel = level;
},
setName(name): void {
loggerName = name;
},
debug(...msg): void {
if (isMoreOrEqualSevere(LogLevel.DEBUG, loggerLevel)) {
loggingFunc(LogLevel.DEBUG, `${loggerName} ${msg.map(m => JSON.stringify(m)).join(' ')}`);
}
},
info(...msg): void {
if (isMoreOrEqualSevere(LogLevel.INFO, loggerLevel)) {
loggingFunc(LogLevel.INFO, `${loggerName} ${msg.map(m => JSON.stringify(m)).join(' ')}`);
}
},
warn(...msg): void {
if (isMoreOrEqualSevere(LogLevel.WARN, loggerLevel)) {
loggingFunc(LogLevel.WARN, `${loggerName} ${msg.map(m => JSON.stringify(m)).join(' ')}`);
}
},
error(...msg): void {
if (isMoreOrEqualSevere(LogLevel.ERROR, loggerLevel)) {
loggingFunc(LogLevel.ERROR, `${loggerName} ${msg.map(m => JSON.stringify(m)).join(' ')}`);
}
},
};

return logger;
}

/**
* INTERNAL determine if a value is a LoggingFunc
*/
export function isLoggingFunc(l: Logger | LoggingFunc): l is LoggingFunc {
return (l as Logger).debug === undefined;
}

/* Helpers for loggerFromLoggingFunc */

/**
* Map of comparable severity values for each log level
*/
const severityByLogLevel: { [key in LogLevel]: number } = {
[LogLevel.ERROR]: 400,
[LogLevel.WARN]: 300,
[LogLevel.INFO]: 200,
[LogLevel.DEBUG]: 100,
};

/**
* Helper to compare two log levels and determine if a is equal or more severe than b
*/
function isMoreOrEqualSevere(a: LogLevel, b: LogLevel): boolean {
return severityByLogLevel[a] >= severityByLogLevel[b];
}

0 comments on commit d67aecf

Please sign in to comment.