Refactor, more tests, updated readme

README.md

@@ -13,16 +13,45 @@ Bristol transport + formatter to send events and errors to Sentry.
 
 ## Installation
 
+You will need to install this package, as well as `raven`, which is the Sentry node.js client.
+
 ```
-npm install --save bristol-sentry
+npm install --save bristol-sentry raven
 ```
 
 ## Getting started
 
-Todo..
+```js
+const bristol = require('bristol');
+const raven = require('raven');
+const bristolSentry = require('bristol-sentry');
+
+const target = bristolSentry({ client: new raven.Client('<your sentry dsn>') });
+
+// Add as a target with the included formatter.
+bristol.addTarget(target).withFormatter(bristolSentry.formatter);
+
+// Try it out
+bristol.debug('here come dat boi');
+bristol.info('watch him rollin watch him go');
+bristol.warn('he be rollin', { rollinWhere: 'down the street' });
+bristol.error(new Error('o shit waddup'));
+```
+
+> **IMPORTANT!** Sentry differentiates between errors and messages. An attempt to log an `Error` object will trigger a `captureException`, whereas anything else triggers a `captureMessage`.
+> Since `capureException` does not store any message other than the one from the error, `bristol-sentry` will add the message (if any) to the `extra` array.
 
 ## Contributing
 
+**You will need a Sentry DSN to run the tests!** Sign up for an account at [Sentry](https://getsentry.com).
+
+You can either add it to your own environment (`SENTRY_DSN`), or you can create an `env.yaml` in the repository root, and add the following:
+
+```yaml
+test:
+  SENTRY_DSN: <your dsn>
+```
+
 Usefull npm run scripts:
 
 * `npm run test`: Runs tests once

lib/bristol-sentry.js

@@ -1,15 +1,7 @@
 'use strict';
-const isString = require('lodash/isString');
-const isNumber = require('lodash/isNumber');
-const isDate = require('lodash/isDate');
-const isBoolean = require('lodash/isBoolean');
-const isRegExp = require('lodash/isRegExp');
-const isError = require('lodash/isError');
-const anyOf = require('./anyOf');
+const sentryFormatter = require('./formatter');
 const bristolSeverityToSentryLevel = require('./bristolSeverityToSentryLevel');
 
-const isFormattable = anyOf([isString, isNumber, isDate, isBoolean, isRegExp]);
-
 /**
  * Factory to make a Bristol target.
  * Installs it's own formatter.
@@ -31,48 +23,15 @@ module.exports = function makeSentryTarget(config) {
     client[method](
       elems.error ? elems.error : elems.message,
       {
-        extra: elems.extra,
+        // If logging an error, we want to add the message to the extra array, so
+        // it does not get lost.
+        extra: elems.error && elems.message ? [elems.message].concat(elems.extra) : elems.extra,
         level: bristolSeverityToSentryLevel(severity)
       }
     );
   }
 
-  /**
-   * Formats log input so we can send it to sentry.
-   */
-  function sentryFormatter(opts, severity, date, elems) {
-    // First item should always be the message.
-    const messageParts = [];
-    let skip = 0;
-    let error;
-    for (const elem of elems) {
-      // Sentry does not allow passing a message to captureException,
-      // so we need to decide here if it's going to be treated
-      // as such or not.
-      if (skip === 0 && isError(elem)) {
-        error = elem;
-        skip++;
-        break;
-      }
-
-      if (!isFormattable(elem)) {
-        break;
-      }
-
-      messageParts.push(elem);
-      skip++;
-    }
-
-    const message = messageParts.join(' ');
-    return {
-      message: message,
-      error: error,
-      extra: elems.slice(skip)
-    };
-  }
+  return logToSentry;
+};
 
-  return {
-    target: logToSentry,
-    formatter: sentryFormatter
-  };
-};
+module.exports.formatter = sentryFormatter;

lib/bristolSeverityToSentryLevel.js

@@ -1,11 +1,12 @@
 'use strict';
 
 const BRISTOL_TO_SENTRY_LEVELS = {
+  fatal: 'fatal',
   error: 'error',
-  warning: 'warn',
+  warning: 'warning',
   info: 'info',
-  debug: 'info',
-  trace: 'info'
+  debug: 'debug',
+  trace: 'debug'
 };
 
 /**

lib/formatter.js

@@ -0,0 +1,43 @@
+'use strict';
+const isString = require('lodash/isString');
+const isNumber = require('lodash/isNumber');
+const isDate = require('lodash/isDate');
+const isBoolean = require('lodash/isBoolean');
+const isRegExp = require('lodash/isRegExp');
+const isError = require('lodash/isError');
+const anyOf = require('./anyOf');
+
+const isFormattable = anyOf([isString, isNumber, isDate, isBoolean, isRegExp]);
+
+/**
+ * Formats log input so we can send it to sentry.
+ */
+function sentryFormatter(opts, severity, date, elems) {
+  // First item should always be the message.
+  const messageParts = [];
+  let skip = 0;
+  let error;
+  for (const elem of elems) {
+    // Sentry does not allow passing a message to captureException,
+    // so we need to decide here if it's going to be treated
+    // as such or not.
+    if (isError(elem)) {
+      error = elem;
+    } else if (isFormattable(elem)) {
+      messageParts.push(elem);
+    } else {
+      break;
+    }
+
+    skip++;
+  }
+
+  const message = messageParts.join(' ');
+  return {
+    message: message,
+    error: error,
+    extra: elems.slice(skip)
+  };
+}
+
+module.exports = sentryFormatter;

package.json

@@ -35,10 +35,10 @@
     "mocha": "^2.4.5",
     "sinon": "^1.17.3",
     "sinon-chai": "^2.8.0",
+    "raven": "^0.11.0",
     "yenv": "^1.0.0"
   },
   "dependencies": {
-    "lodash": "^4.13.1",
-    "raven": "^0.11.0"
+    "lodash": "^4.13.1"
   }
 }

test/lib/bristol-sentry.spec.js

@@ -2,6 +2,7 @@
 const bristolSentry = require('../../lib/bristol-sentry');
 const bristol = require('bristol');
 const raven = require('raven');
+const formatter = bristolSentry.formatter;
 
 describe('bristol-sentry', function() {
   it('exists', function() {
@@ -10,31 +11,41 @@ describe('bristol-sentry', function() {
 
   it('creates an object that has a formatter and a target', function() {
     const subject = bristolSentry({ });
-    subject.should.be.an.object;
-    subject.target.should.be.a.function;
-    subject.formatter.should.be.a.function;
+    subject.should.be.a.function;
+    formatter.should.be.a.function;
   });
 
   describe('formatter', function() {
     it('returns an object with a message and extras', function() {
       const subject = bristolSentry({ });
-      const result = subject.formatter({}, 'error', new Date(), ['Some', 'message', 123, 'cool', {some: 'object'}]);
+      const result = formatter({}, 'error', new Date(), ['Some', 'message', 123, 'cool', {some: 'object'}]);
       result.message.should.equal('Some message 123 cool');
       result.extra[0].some.should.equal('object');
     });
 
-    it('returns an object with error set if first elem is an error', function() {
+    it('returns an object with error set if an elem is an error', function() {
       const error = new Error();
       const subject = bristolSentry({ });
-      const result = subject.formatter({}, 'error', new Date(), [error, 'nope']);
+      const result = formatter({}, 'error', new Date(), [error, 'nope']);
       result.error.should.equal(error);
+      result.message.should.equal('nope');
     });
 
-    it('returns an object with no error set if elem is an error but not the first', function() {
+    it('returns an object with an error set if elem is an error even if not the first', function() {
       const error = new Error();
       const subject = bristolSentry({ });
-      const result = subject.formatter({}, 'error', new Date(), ['nope', error]);
-      expect(result.error).to.be.undefined;
+      const result = formatter({}, 'error', new Date(), ['nope', error]);
+      result.error.should.equal(error);
+      result.message.should.equal('nope');
+    });
+
+    it('handles funky ordering', function() {
+      const error = new Error('dat boi');
+      const subject = bristolSentry({ });
+      const result = formatter({}, 'error', new Date(), ['here come', error.message, error, 'shit waddup', { rollin: 'down the street' }]);
+      result.error.should.equal(error);
+      result.message.should.equal('here come dat boi shit waddup');
+      result.extra[0].rollin.should.equal('down the street');
     });
   });
 
@@ -51,15 +62,22 @@ describe('bristol-sentry', function() {
     });
 
     it('calls captureMessage when there is no error', function() {
-      subject.target({}, 'error', new Date(), { message: 'Hello!', extra: { hello: 'world' }});
+      subject({}, 'error', new Date(), { message: 'Hello!', extra: { hello: 'world' }});
       client.captureMessage.should.have.been.calledWith('Hello!', { level: 'error', extra: { hello: 'world' }});
     });
 
     it('calls captureException when there is an error', function() {
       const error = new Error();
-      subject.target({}, 'error', new Date(), { error: error, extra: [{ hello: 'world' }]});
+      subject({}, 'error', new Date(), { error: error, extra: [{ hello: 'world' }]});
       client.captureException.should.have.been.calledWith(error, { level: 'error', extra: [{ hello: 'world' }]});
     });
+
+    it('adds message to extra when capturing error', function() {
+      const error = new Error('aww yiss');
+      const message = 'mother fuckin bread crumbs';
+      subject({}, 'error', new Date(), { message: message, error: error, extra: [{ hello: 'world' }]});
+      client.captureException.should.have.been.calledWith(error, { level: 'error', extra: [message, { hello: 'world' }]});
+    });
   });
 
   describe('with bristol', function() {
@@ -73,7 +91,7 @@ describe('bristol-sentry', function() {
 
       subject = bristolSentry({ client: client });
       logger = new bristol.Bristol();
-      logger.addTarget(subject.target).withFormatter(subject.formatter);
+      logger.addTarget(subject).withFormatter(formatter);
     });
 
     it('calls the correct client methods', function() {
@@ -89,7 +107,7 @@ describe('bristol-sentry', function() {
     it('does not fail, heh', function() {
       const logger = new bristol.Bristol();
       const bs = bristolSentry({ client: new raven.Client(process.env.SENTRY_DSN)});
-      logger.addTarget(bs.target).withFormatter(bs.formatter);
+      logger.addTarget(bs).withFormatter(formatter);
 
       logger.debug('Debug log', 42, { more: 'stuff' });
       logger.info('Info log', { more: 'stuff' });

test/lib/bristolSeverityToSentryLevel.spec.js

@@ -3,11 +3,12 @@ const bristolSeverityToSentryLevel = require('../../lib/bristolSeverityToSentryL
 
 describe('bristolSeverityToSentryLevel', function() {
   it('converts levels correctly ', function() {
+    bristolSeverityToSentryLevel('fatal').should.equal('fatal');
     bristolSeverityToSentryLevel('error').should.equal('error');
-    bristolSeverityToSentryLevel('warning').should.equal('warn');
+    bristolSeverityToSentryLevel('warning').should.equal('warning');
     bristolSeverityToSentryLevel('info').should.equal('info');
-    bristolSeverityToSentryLevel('debug').should.equal('info');
-    bristolSeverityToSentryLevel('trace').should.equal('info');
+    bristolSeverityToSentryLevel('debug').should.equal('debug');
+    bristolSeverityToSentryLevel('trace').should.equal('debug');
     bristolSeverityToSentryLevel('unknown').should.equal('error');
   });
 });