Skip to content

Commit

Permalink
Updates the logger usage so that it follows the patter the JSDoc clai…
Browse files Browse the repository at this point in the history
…ms it does.

Refactors the logger creation out to a getLogger() function so data-store and client can use a single codepath.

Fixes #146
  • Loading branch information
l12s committed Feb 28, 2016
1 parent 8482f78 commit 2576ee8
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### v2.0.3 (TBD)

* The RTM `AUTHENTICATED` event now also emits the `rtm.start` payload
* Fixes the way that loggers are instantiated and used, so that the JSDoc for `opts.logger` is correct

### v2.0.2 (2016-02-15)

Expand Down
12 changes: 4 additions & 8 deletions lib/clients/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@
*
*/

var ConsoleTransport = require('winston').transports.Console;
var EventEmitter = require('eventemitter3');
var async = require('async');
var bind = require('lodash').bind;
var inherits = require('inherits');
var retry = require('retry');
var urlJoin = require('url-join');
var winston = require('winston');

var WEB_CLIENT_EVENTS = require('./events/client').WEB;
var getLogger = require('../helpers').getLogger;
var helpers = require('./helpers');
var requestsTransport = require('./transports/request').requestTransport;

Expand Down Expand Up @@ -61,10 +60,7 @@ function BaseAPIClient(token, opts) {
* The logger function attached to this client.
* @type {Function}
*/
this.logger = opts.logger || new winston.Logger({
level: opts.logLevel || 'info',
transports: [new ConsoleTransport()],
});
this.logger = opts.logger || getLogger();

this._createFacets();
}
Expand All @@ -74,7 +70,7 @@ inherits(BaseAPIClient, EventEmitter);

BaseAPIClient.prototype.emit = function emit() {
BaseAPIClient.super_.prototype.emit.apply(this, arguments);
this.logger.debug(arguments);
this.logger('debug', arguments);
};


Expand Down Expand Up @@ -173,7 +169,7 @@ BaseAPIClient.prototype._callTransport = function _callTransport(task, queueCb)
cb(jsonParseErr, jsonResponse);
} catch (callbackErr) {
// Never retry requests that fail in the callback
_this.logger.error(callbackErr);
_this.logger('error', callbackErr);
}
}

Expand Down
20 changes: 10 additions & 10 deletions lib/clients/rtm/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ RTMClient.prototype._createFacets = function _createFacets() {
RTMClient.prototype.start = function start(opts) {
// Check whether the client is currently attempting to connect to the RTM API.
if (!this._connecting) {
this.logger.log('verbose', 'attempting to connect via the RTM API');
this.logger('verbose', 'attempting to connect via the RTM API');
this.emit(CLIENT_EVENTS.CONNECTING);
this._connecting = true;

Expand Down Expand Up @@ -223,17 +223,17 @@ RTMClient.prototype._onStart = function _onStart(err, data) {
// Any of these mean this client is unusable, so don't attempt to auto-reconnect
if (data && contains(UNRECOVERABLE_RTM_START_ERRS, data.error)) {
errMsg = 'unrecoverable failure connecting to the RTM API';
this.logger.error(errMsg + ': ' + data.error);
this.logger('error', errMsg + ': ' + data.error);
this.disconnect(errMsg, data.error);
} else {
this.logger.info('unable to RTM start, attempting reconnect: ' + err || data.error);
this.logger('info', 'unable to RTM start, attempting reconnect: ' + err || data.error);
this.authenticated = false;
if (this.autoReconnect) {
this.reconnect();
}
}
} else {
this.logger.verbose('rtm.start successful, attempting to open websocket URL');
this.logger('verbose', 'rtm.start successful, attempting to open websocket URL');
this.authenticated = true;
this.activeUserId = data.self.id;
this.activeTeamId = data.team.id;
Expand Down Expand Up @@ -356,14 +356,14 @@ RTMClient.prototype.handleWsOpen = function handleWsOpen() {
*/
RTMClient.prototype.handleWsMessage = function handleWsMessage(wsMsg) {
var message;
this.logger.log('debug', wsMsg);
this.logger('debug', wsMsg);
this.emit(CLIENT_EVENTS.RAW_MESSAGE, wsMsg);

try {
message = JSON.parse(wsMsg);
} catch (err) {
// TODO(leah): Emit an event here?
this.logger.debug('unable to parse message: ' + err);
this.logger('debug', 'unable to parse message: ' + err);
return;
}

Expand Down Expand Up @@ -434,7 +434,7 @@ RTMClient.prototype._handleWsMessageViaEventHandler = function _handleWsMessageV
* @param {Object} err
*/
RTMClient.prototype.handleWsError = function handleWsError(err) {
this.logger.debug(err);
this.logger('debug', err);
this.emit(CLIENT_EVENTS.WS_ERROR, err);
};

Expand Down Expand Up @@ -514,12 +514,12 @@ RTMClient.prototype.send = function send(message, optCb) {
if (this.connected) {
wsMsg.id = this.nextMessageId();
jsonMessage = JSON.stringify(wsMsg);
this.logger.log('debug', jsonMessage);
this.logger('debug', jsonMessage);

this._pendingMessages[wsMsg.id] = wsMsg;
this.ws.send(jsonMessage, undefined, function handleWsMsgResponse(wsRespErr) {
if (!isUndefined(wsRespErr)) {
_this.logger.debug('Unable to send message: ' + wsRespErr);
_this.logger('debug', 'Unable to send message: ' + wsRespErr);
}

if (!isUndefined(optCb)) {
Expand All @@ -528,7 +528,7 @@ RTMClient.prototype.send = function send(message, optCb) {
});
} else {
err = 'ws not connected, unable to send message';
this.logger.debug(err);
this.logger('debug', err);
if (!isUndefined(optCb)) {
optCb(new Error(err));
}
Expand Down
11 changes: 4 additions & 7 deletions lib/data-store/data-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
* Interface for creating a data store object for caching information from the Slack APIs.
*/

var ConsoleTransport = require('winston').transports.Console;
var bind = require('lodash').bind;
var forEach = require('lodash').forEach;
var isUndefined = require('lodash').isUndefined;
var winston = require('winston');

var RTM_API_EVENTS = require('../clients/events/rtm').EVENTS;
var getLogger = require('../helpers').getLogger;
var makeMessageEventWithSubtype = require('../clients/events/utils').makeMessageEventWithSubtype;
var messageHandlers = require('./message-handlers');
var models = require('../models');
Expand All @@ -29,10 +29,7 @@ function SlackDataStore(opts) {
* The logger function attached to this client.
* @type {Function}
*/
this.logger = dataStoreOpts.logger || new winston.Logger({
level: dataStoreOpts.logLevel || 'info',
transports: [new ConsoleTransport()],
});
this.logger = dataStoreOpts.logger || getLogger();

forEach(messageHandlers, function anonRegisterMessageHandler(handler, event) {
this.registerMessageHandler(event, handler);
Expand Down Expand Up @@ -431,7 +428,7 @@ SlackDataStore.prototype.handleRtmMessage = function handleRtmMessage(
}
} catch (err) {
// TODO(leah): Do something more here?
this.logger.debug(err);
this.logger('debug', err);
}
}
};
Expand Down
25 changes: 25 additions & 0 deletions lib/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Top level helpers.
*/

var ConsoleTransport = require('winston').transports.Console;
var bind = require('lodash').bind;
var winston = require('winston');


/**
*
* @param {string} optLogLevel
* @param {Object} optTransport
* @returns {function(this:*)|Function|*}
*/
var getLogger = function getLogger(optLogLevel, optTransport) {
var logger = new winston.Logger({
level: optLogLevel || 'info',
transports: [optTransport || new ConsoleTransport()],
});
return bind(logger.log, logger);
};


module.exports.getLogger = getLogger;

0 comments on commit 2576ee8

Please sign in to comment.