diff --git a/.eslintrc b/.eslintrc index a44793c7c..3a014f237 100644 --- a/.eslintrc +++ b/.eslintrc @@ -1,3 +1,6 @@ { - "extends": "airbnb/legacy" + "extends": "airbnb/legacy", + "rules": { + "max-len": ["error", 100, { "ignoreComments": true }] + } } diff --git a/lib/clients/client.js b/lib/clients/client.js index 17313c474..fe82ec3ff 100644 --- a/lib/clients/client.js +++ b/lib/clients/client.js @@ -14,6 +14,7 @@ var SlackAPIError = require('./errors').SlackAPIError; var callTransport = require('./transports/call-transport'); var getLogger = require('../helpers').getLogger; var helpers = require('./helpers'); +var globalHelpers = require('../helpers'); var requestsTransport = require('./transports/request').requestTransport; var retryPolicies = require('./retry-policies'); @@ -23,7 +24,6 @@ var retryPolicies = require('./retry-policies'); * @param {string} token The Slack API token to use with this client. * @param {Object} opts * @param {String} opts.slackAPIUrl The Slack API URL. - * @param {String} opts.userAgent The user-agent to use, defaults to node-slack. * @param {Function} opts.transport Function to call to make an HTTP call to the Slack API. * @param {string=} opts.logLevel The log level for the logger. * @param {Function=} opts.logger Function to use for log calls, takes (logLevel, logString) params. @@ -49,9 +49,6 @@ function BaseAPIClient(token, opts) { /** @type {Function} */ this.transport = clientOpts.transport || requestsTransport; - /** @type {string} */ - this.userAgent = clientOpts.userAgent || 'node-slack'; - /** * Default to attempting 5 retries within 5 minutes, with exponential backoff. */ @@ -78,7 +75,6 @@ function BaseAPIClient(token, opts) { inherits(BaseAPIClient, EventEmitter); - BaseAPIClient.prototype.emit = function emit() { BaseAPIClient.super_.prototype.emit.apply(this, arguments); this.logger('debug', arguments); @@ -138,7 +134,14 @@ BaseAPIClient.prototype._callTransport = function _callTransport(task, queueCb) */ BaseAPIClient.prototype._makeAPICall = function _makeAPICall(endpoint, apiArgs, apiOptArgs, optCb) { var apiCallArgs = helpers.getAPICallArgs( - this._token, this.userAgent, this.slackAPIUrl, endpoint, apiArgs, apiOptArgs, optCb); + this._token, + globalHelpers.getVersionString(), + this.slackAPIUrl, + endpoint, + apiArgs, + apiOptArgs, + optCb + ); var cb = apiCallArgs.cb; var args = apiCallArgs.args; var promise; diff --git a/lib/clients/rtm/client.js b/lib/clients/rtm/client.js index b0e0b8db3..1dabb3359 100644 --- a/lib/clients/rtm/client.js +++ b/lib/clients/rtm/client.js @@ -36,7 +36,6 @@ var SlackRTMSendError = require('../errors').SlackRTMSendError; var makeMessageEventWithSubtype = require('../events/utils').makeMessageEventWithSubtype; var wsSocketFn = require('../transports/ws'); - /** * * @param {String} token @@ -518,7 +517,7 @@ RTMClient.prototype._handleMessageAck = function handleMessageAck(replyTo, messa } else { // This should be impossible. If the message is received with no message type, it's a response // to a message sent by this client, so the absence of a handler for it should never happen. - this.logger('error', 'message received with unknown reply_to: ' + message); + this.logger('error', 'message received with unknown reply_to: ' + JSON.stringify(message)); } }; @@ -556,7 +555,8 @@ RTMClient.prototype._handleMostRecentMsgReply = function _handleMostRecentMsgRep this._handleMsgResponse(replyTo, null, msg); } else { // The only time this should happen is when a client first connects - this.logger('info', 'message received on reconnect with no registered callback:\n' + message); + this.logger('info', 'message received on reconnect with no registered callback:\n' + + JSON.stringify(message)); } pendingMessageIds = keys(this._pendingMessages); diff --git a/lib/clients/transports/ws.js b/lib/clients/transports/ws.js index 0ce8985bd..440dbf57f 100644 --- a/lib/clients/transports/ws.js +++ b/lib/clients/transports/ws.js @@ -4,7 +4,7 @@ var HttpsProxyAgent = require('https-proxy-agent'); var WebSocket = require('ws'); - +var globalHelpers = require('../../helpers'); /** * @@ -23,6 +23,10 @@ var wsTransport = function wsTransport(socketUrl, opts) { wsOpts.agent = new HttpsProxyAgent(proxyURL); } + wsOpts.headers = { + 'User-Agent': globalHelpers.getVersionString() + }; + return new WebSocket(socketUrl, wsOpts); }; diff --git a/lib/helpers.js b/lib/helpers.js index 26cef7e62..dbd7dd858 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -5,6 +5,8 @@ var ConsoleTransport = require('winston').transports.Console; var bind = require('lodash').bind; var winston = require('winston'); +var os = require('os'); +var pkginfo = require('pkginfo')(module, 'version', 'repository'); // eslint-disable-line no-unused-vars /** @@ -21,5 +23,40 @@ var getLogger = function getLogger(optLogLevel, optTransport) { return bind(logger.log, logger); }; +// TODO we can do this better +var versionStr = module.exports.repository.replace('/', ':') + '/' + module.exports.version + ' ' + + os.platform() + '/' + os.release() + ' ' + + 'node/' + process.version.replace('v', ''); + +var getVersionString = function getVersionString() { + return versionStr; +}; + +/** + * + * A function to append information to the end of the informational string. You can append an + * arbitrary string in the first parameter (although all `/` characters will be replaced with + * `:` characters. You can provide an optional string for the version of the resource as well, when + * this makes sense. + * + * In general, you should use this function to let us know what you are building with this SDK. If + * you are building a bot called "FooBot", and are currently on version "1.0", then call the function + * as `appendToVersionString('FooBot', '1.0') to have that information sent to Slack during + * interactions with the Slack Platform + * + * @param {string} name + * @param {string} [version] + * + */ +var appendToVersionString = function appendToVersionString(name, version) { + var correctName = name.replace('/', ':'); + if (version) { + versionStr = versionStr + ' ' + correctName + '/' + version.replace('/', ':'); + } else { + versionStr = versionStr + ' ' + correctName; + } +}; module.exports.getLogger = getLogger; +module.exports.getVersionString = getVersionString; +module.exports.appendToVersionString = appendToVersionString; diff --git a/package.json b/package.json index 641e7e44c..c8ed47cba 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "https-proxy-agent": "^1.0.0", "inherits": "^2.0.1", "lodash": "^4.13.1", + "pkginfo": "^0.4.0", "request": "^2.64.0", "retry": "^0.9.0", "url-join": "0.0.1", diff --git a/test/.eslintrc b/test/.eslintrc index 6541724a6..1e5c85011 100644 --- a/test/.eslintrc +++ b/test/.eslintrc @@ -9,6 +9,7 @@ "no-var": 0, "object-shorthand": 0, "padded-blocks": 0, - "func-names": 0 + "func-names": 0, + "max-len": ["error", 100, { "ignoreComments": true }] } } diff --git a/test/clients/helpers.js b/test/clients/helpers.js index 3f78edcce..08343ac95 100644 --- a/test/clients/helpers.js +++ b/test/clients/helpers.js @@ -87,6 +87,5 @@ describe('Client Helpers', function () { expect(args.cb).to.equal(null); expect(args.args.data).to.deep.equal({ test: '1', token: 't' }); }); - }); }); diff --git a/test/clients/web/client.js b/test/clients/web/client.js index 4d0d7ed18..016df179c 100644 --- a/test/clients/web/client.js +++ b/test/clients/web/client.js @@ -4,6 +4,7 @@ var path = require('path'); var lodash = require('lodash'); var nock = require('nock'); var sinon = require('sinon'); +var pkginfo = require('pkginfo')(module, 'version', 'repository'); // eslint-disable-line no-unused-vars var WebAPIClient = require('../../../lib/clients/web/client'); var retryPolicies = require('../../../lib/clients/retry-policies'); @@ -42,13 +43,11 @@ describe('Web API Client', function () { it('should accept supplied defaults when present', function () { var opts = { slackAPIUrl: 'test', - userAgent: 'test', transport: lodash.noop }; var client = new WebAPIClient('test-token', opts); expect(client.slackAPIUrl).to.equal('test'); - expect(client.userAgent).to.equal('test'); }); it('should register facets during construction', function () { diff --git a/test/helpers.js b/test/helpers.js new file mode 100644 index 000000000..90af1d656 --- /dev/null +++ b/test/helpers.js @@ -0,0 +1,46 @@ +var expect = require('chai').expect; +var helpers = require('../lib/helpers'); +var os = require('os'); +var pkginfo = require('pkginfo')(module, 'version', 'repository'); // eslint-disable-line no-unused-vars + +var userAgent = module.exports.repository.replace('/', ':') + '/' + module.exports.version + + ' ' + os.platform() + '/' + os.release() + + ' node/' + process.version.replace('v', ''); + +describe('Version strings', function () { + describe('getVersionString', function () { + it('returns the current version, when unmodified', function () { + var str = helpers.getVersionString(); + expect(str).to.equal(userAgent); + }); + }); + + describe('appendToVersionString', function () { + it('appends a new value', function () { + var str; + helpers.appendToVersionString('foobar', '1.2'); + str = helpers.getVersionString(); + expect(str).to.equal(userAgent + ' foobar/1.2'); + }); + }); + + describe('appendToVersionString', function () { + it('should never overwrite an existing value', function () { + var str; + helpers.appendToVersionString('foobar', '1.3'); + str = helpers.getVersionString(); + // TODO that the order of these tests _matters_ bothers me + expect(str).to.equal(userAgent + ' foobar/1.2 foobar/1.3'); + }); + }); + + describe('appendToVersionStringWithoutVersion', function () { + it('should never overwrite an existing value', function () { + var str; + helpers.appendToVersionString('foobar'); + str = helpers.getVersionString(); + // TODO that the order of these tests _matters_ bothers me + expect(str).to.equal(userAgent + ' foobar/1.2 foobar/1.3 foobar'); + }); + }); +});