Rework to favor tagging over wrapping

libraries/o-error/index.js

@@ -1,32 +1,32 @@
-'use strict'
-
 /**
- * Make custom error types that pass `instanceof` checks, have stack traces,
- * support custom messages and properties, and support wrapping errors (causes).
- *
- * @module
- */
-
-/**
- * A base class for custom errors that handles:
- *
- * 1. Wrapping an optional 'cause'.
- * 2. Storing an 'info' object with additional data.
- * 3. Setting the name to the subclass name.
- *
- * @extends Error
+ * Light-weight helpers for handling JavaScript Errors in node.js and the
+ * browser. {@see README.md}
  */
 class OError extends Error {
   /**
    * @param {string} message as for built-in Error
-   * @param {?object} info extra data to attach to the error
+   * @param {Object} [info] extra data to attach to the error
+   * @param {Error} [cause]
    */
-  constructor(message, info) {
+  constructor(message, info, cause) {
     super(message)
     this.name = this.constructor.name
-    if (info) {
-      this.info = info
+    if (info) this.info = info
+    if (cause) this.cause = cause
+
+    /** @type {Array<TaggedError>} */
+    this._oErrorTags // eslint-disable-line
   }
+
+  /**
+   * Set the extra info object for this error.
+   *
+   * @param {Object | null | undefined} info
+   * @return {this}
+   */
+  withInfo(info) {
+    this.info = info
+    return this
   }
 
   /**
@@ -37,57 +37,98 @@ class OError extends Error {
    */
   withCause(cause) {
     this.cause = cause
-    if (this.message && cause.message) {
-      this.message += ': ' + cause.message
-    }
     return this
   }
-}
 
   /**
- * Return the `info` property from `error` and recursively merge the `info`
- * properties from the error's causes, if any.
+   * Tag debugging information onto any error (whether an OError or not) and
+   * return it.
    *
- * If a property is repeated, the first one in the cause chain wins.
+   * @param {Error} error
+   * @param {string} [message]
+   * @param {Object} [info]
+   * @return {Error} the modified `error` argument
+   */
+  static tag(error, message, info) {
+    const oError = /** @type{OError} */ (error)
+
+    if (!oError._oErrorTags) oError._oErrorTags = []
+
+    const tag = new TaggedError(message, info)
+
+    // Hide this function in the stack trace.
+    if (Error.captureStackTrace) Error.captureStackTrace(tag, OError.tag)
+
+    oError._oErrorTags.push(tag)
+
+    return error
+  }
+
+  /**
+   * The merged info from any `tag`s on the given error.
    *
- * @param {?Error} error assumed not to have circular causes
+   * If an info property is repeated, the last one wins.
+   *
+   * @param {Error} error
    * @return {Object}
    */
-function getFullInfo(error) {
-  if (!error) return {}
-  const info = getFullInfo(error.cause)
-  if (typeof error.info === 'object') Object.assign(info, error.info)
+  static getFullInfo(error) {
+    const info = {}
+
+    if (!error) return info
+
+    const oError = /** @type{OError} */ (error)
+
+    if (typeof oError.info === 'object') Object.assign(info, oError.info)
+
+    if (oError._oErrorTags) {
+      for (const tag of oError._oErrorTags) {
+        Object.assign(info, tag.info)
+      }
+    }
+
     return info
   }
 
   /**
- * Return the `stack` property from `error` and recursively append the `stack`
- * properties from the error's causes, if any.
+   * Return the `stack` property from `error`, including the `stack`s for any
+   * tagged errors added with `OError.tag` and for any `cause`s.
    *
- * @param {?Error} error assumed not to have circular causes
+   * @param {Error | null | undefined} error
    * @return {string}
    */
-function getFullStack(error) {
+  static getFullStack(error) {
     if (!error) return ''
-  const causeStack = getFullStack(error.cause)
-  if (causeStack) return error.stack + '\ncaused by: ' + causeStack
-  return error.stack
+
+    const oError = /** @type{OError} */ (error)
+
+    let stack = oError.stack
+
+    if (oError._oErrorTags) {
+      for (const tag of oError._oErrorTags) {
+        stack += `\n${tag.stack}`
+      }
+    }
+
+    const causeStack = oError.cause && OError.getFullStack(oError.cause)
+    if (causeStack) {
+      stack += '\ncaused by:\n' + indent(causeStack)
+    }
+
+    return stack
+  }
 }
 
 /**
- * Is `error` or one of its causes an instance of `klass`?
+ * Used to record a stack trace every time we tag info onto an Error.
  *
- * @param  {?Error} error assumed not to have circular causes
- * @param  {function} klass
- * @return {Boolean}
+ * @private
+ * @extends OError
  */
-function hasCauseInstanceOf(error, klass) {
-  if (!error) return false
-  return error instanceof klass || hasCauseInstanceOf(error.cause, klass)
+class TaggedError extends OError {}
+
+function indent(string) {
+  return string.replace(/^/gm, '    ')
 }
 
-OError.getFullInfo = getFullInfo
-OError.getFullStack = getFullStack
-OError.hasCauseInstanceOf = hasCauseInstanceOf
-
 module.exports = OError

libraries/o-error/test/o-error-util.test.js

@@ -1,53 +1,201 @@
-const { getFullInfo, getFullStack, hasCauseInstanceOf } = require('..')
 const { expect } = require('chai')
+const { promisify } = require('util')
+
+const OError = require('..')
+
+const {
+  expectError,
+  expectFullStackWithoutStackFramesToEqual,
+} = require('./support')
+
+describe('OError.tag', function () {
+  it('tags errors thrown from an async function', async function () {
+    const delay = promisify(setTimeout)
+
+    async function foo() {
+      await delay(10)
+      throw new Error('foo error')
+    }
+
+    async function bar() {
+      try {
+        await foo()
+      } catch (error) {
+        throw OError.tag(error, 'failed to bar', { bar: 'baz' })
+      }
+    }
+
+    async function baz() {
+      try {
+        await bar()
+      } catch (error) {
+        throw OError.tag(error, 'failed to baz', { baz: 'bat' })
+      }
+    }
+
+    try {
+      await baz()
+      expect.fail('should have thrown')
+    } catch (error) {
+      expectError(error, {
+        name: 'Error',
+        klass: Error,
+        message: 'Error: foo error',
+        firstFrameRx: /at foo/,
+      })
+      expectFullStackWithoutStackFramesToEqual(error, [
+        'Error: foo error',
+        'TaggedError: failed to bar',
+        'TaggedError: failed to baz',
+      ])
+      expect(OError.getFullInfo(error)).to.eql({
+        bar: 'baz',
+        baz: 'bat',
+      })
+    }
+  })
+
+  it('tags errors thrown from a promise rejection', async function () {
+    function foo() {
+      return new Promise((resolve, reject) => {
+        setTimeout(function () {
+          reject(new Error('foo error'))
+        }, 10)
+      })
+    }
+
+    async function bar() {
+      try {
+        await foo()
+      } catch (error) {
+        throw OError.tag(error, 'failed to bar', { bar: 'baz' })
+      }
+    }
+
+    async function baz() {
+      try {
+        await bar()
+      } catch (error) {
+        throw OError.tag(error, 'failed to baz', { baz: 'bat' })
+      }
+    }
+
+    try {
+      await baz()
+      expect.fail('should have thrown')
+    } catch (error) {
+      expectError(error, {
+        name: 'Error',
+        klass: Error,
+        message: 'Error: foo error',
+        firstFrameRx: /_onTimeout/,
+      })
+      expectFullStackWithoutStackFramesToEqual(error, [
+        'Error: foo error',
+        'TaggedError: failed to bar',
+        'TaggedError: failed to baz',
+      ])
+      expect(OError.getFullInfo(error)).to.eql({
+        bar: 'baz',
+        baz: 'bat',
+      })
+    }
+  })
+
+  it('tags errors yielded through callbacks', function (done) {
+    function foo(cb) {
+      setTimeout(function () {
+        cb(new Error('foo error'))
+      }, 10)
+    }
+
+    function bar(cb) {
+      foo(function (err) {
+        if (err) {
+          return cb(OError.tag(err, 'failed to bar', { bar: 'baz' }))
+        }
+        cb()
+      })
+    }
+
+    function baz(cb) {
+      bar(function (err) {
+        if (err) {
+          return cb(OError.tag(err, 'failed to baz', { baz: 'bat' }))
+        }
+        cb()
+      })
+    }
+
+    baz(function (err) {
+      if (err) {
+        expectError(err, {
+          name: 'Error',
+          klass: Error,
+          message: 'Error: foo error',
+          firstFrameRx: /_onTimeout/,
+        })
+        expectFullStackWithoutStackFramesToEqual(err, [
+          'Error: foo error',
+          'TaggedError: failed to bar',
+          'TaggedError: failed to baz',
+        ])
+        expect(OError.getFullInfo(err)).to.eql({
+          bar: 'baz',
+          baz: 'bat',
+        })
+        return done()
+      }
+      expect.fail('should have yielded an error')
+    })
+  })
+})
 
 describe('OError.getFullInfo', function () {
   it('works on a normal error', function () {
     const err = new Error('foo')
-    expect(getFullInfo(err)).to.deep.equal({})
+    expect(OError.getFullInfo(err)).to.deep.equal({})
   })
 
-  it('works on an error with .info', function () {
-    const err = new Error('foo')
-    err.info = { userId: 123 }
-    expect(getFullInfo(err)).to.deep.equal({ userId: 123 })
+  it('works on an error with tags', function () {
+    const err = OError.tag(new Error('foo'), 'bar', { userId: 123 })
+    expect(OError.getFullInfo(err)).to.deep.equal({ userId: 123 })
   })
 
-  it('merges info from a cause chain', function () {
+  it('merges info from an error and its tags', function () {
+    const err = new OError('foo').withInfo({ projectId: 456 })
+    OError.tag(err, 'failed to foo', { userId: 123 })
+    expect(OError.getFullInfo(err)).to.deep.equal({
+      projectId: 456,
+      userId: 123,
+    })
+  })
+
+  it('does not merge info from a cause', function () {
     const err1 = new Error('foo')
     const err2 = new Error('bar')
     err1.cause = err2
     err2.info = { userId: 123 }
-    expect(getFullInfo(err1)).to.deep.equal({ userId: 123 })
+    expect(OError.getFullInfo(err1)).to.deep.equal({})
   })
 
-  it('merges info from a cause chain with no info', function () {
-    const err1 = new Error('foo')
-    const err2 = new Error('bar')
-    err1.cause = err2
-    expect(getFullInfo(err1)).to.deep.equal({})
-  })
-
-  it('merges info from a cause chain with duplicate keys', function () {
-    const err1 = new Error('foo')
-    const err2 = new Error('bar')
-    err1.cause = err2
-    err1.info = { userId: 123 }
-    err2.info = { userId: 456 }
-    expect(getFullInfo(err1)).to.deep.equal({ userId: 123 })
+  it('merges info from tags with duplicate keys', function () {
+    const err1 = OError.tag(new Error('foo'), 'bar', { userId: 123 })
+    const err2 = OError.tag(err1, 'bat', { userId: 456 })
+    expect(OError.getFullInfo(err2)).to.deep.equal({ userId: 456 })
   })
 
   it('works on an error with .info set to a string', function () {
     const err = new Error('foo')
     err.info = 'test'
-    expect(getFullInfo(err)).to.deep.equal({})
+    expect(OError.getFullInfo(err)).to.deep.equal({})
   })
 })
 
 describe('OError.getFullStack', function () {
   it('works on a normal error', function () {
     const err = new Error('foo')
-    const fullStack = getFullStack(err)
+    const fullStack = OError.getFullStack(err)
     expect(fullStack).to.match(/^Error: foo$/m)
     expect(fullStack).to.match(/^\s+at /m)
   })
@@ -57,28 +205,62 @@ describe('OError.getFullStack', function () {
     const err2 = new Error('bar')
     err1.cause = err2
 
-    const fullStack = getFullStack(err1)
+    const fullStack = OError.getFullStack(err1)
     expect(fullStack).to.match(/^Error: foo$/m)
     expect(fullStack).to.match(/^\s+at /m)
-    expect(fullStack).to.match(/^caused by: Error: bar$/m)
-  })
+    expect(fullStack).to.match(/^caused by:\n\s+Error: bar$/m)
   })
 
-describe('OError.hasCauseInstanceOf', function () {
-  it('works on a normal error', function () {
-    const err = new Error('foo')
-    expect(hasCauseInstanceOf(null, Error)).to.be.false
-    expect(hasCauseInstanceOf(err, Error)).to.be.true
-    expect(hasCauseInstanceOf(err, RangeError)).to.be.false
+  it('works on both tags and causes', async function () {
+    // Here's the actual error.
+    function tryToFoo() {
+      try {
+        throw Error('foo')
+      } catch (error) {
+        throw OError.tag(error, 'failed to foo', { foo: 1 })
+      }
+    }
+
+    // Inside another function that wraps it.
+    function tryToBar() {
+      try {
+        tryToFoo()
+      } catch (error) {
+        throw new OError('failed to bar').withCause(error)
+      }
+    }
+
+    // And it is in another try.
+    try {
+      try {
+        tryToBar()
+        expect.fail('should have thrown')
+      } catch (error) {
+        throw OError.tag(error, 'failed to bat', { bat: 1 })
+      }
+    } catch (error) {
+      // We catch the wrapping error.
+      expectError(error, {
+        name: 'OError',
+        klass: OError,
+        message: 'OError: failed to bar',
+        firstFrameRx: /tryToBar/,
       })
 
-  it('works on an error with a cause', function () {
-    const err1 = new Error('foo')
-    const err2 = new RangeError('bar')
-    err1.cause = err2
+      // But the stack contains all of the errors and tags.
+      expectFullStackWithoutStackFramesToEqual(error, [
+        'OError: failed to bar',
+        'TaggedError: failed to bat',
+        'caused by:',
+        '    Error: foo',
+        '    TaggedError: failed to foo',
+      ])
 
-    expect(hasCauseInstanceOf(err1, Error)).to.be.true
-    expect(hasCauseInstanceOf(err1, RangeError)).to.be.true
-    expect(hasCauseInstanceOf(err1, TypeError)).to.be.false
+      // The info from the wrapped cause should not leak out.
+      expect(OError.getFullInfo(error)).to.eql({ bat: 1 })
+
+      // But it should still be recorded.
+      expect(OError.getFullInfo(error.cause)).to.eql({ foo: 1 })
+    }
   })
 })

libraries/o-error/test/o-error.test.js

@@ -1,21 +1,40 @@
 const { expect } = require('chai')
 
 const OError = require('..')
-const { expectError } = require('./support')
+const {
+  expectError,
+  expectFullStackWithoutStackFramesToEqual,
+} = require('./support')
 
 class CustomError1 extends OError {
-  constructor(info) {
-    super('failed to foo', info)
+  constructor() {
+    super('failed to foo')
   }
 }
 
 class CustomError2 extends OError {
-  constructor(customMessage, info) {
-    super(customMessage || 'failed to bar', info)
+  constructor(customMessage) {
+    super(customMessage || 'failed to bar')
   }
 }
 
 describe('OError', function () {
+  it('can have an info object', function () {
+    const err1 = new OError('foo', { foo: 1 })
+    expect(err1.info).to.eql({ foo: 1 })
+
+    const err2 = new OError('foo').withInfo({ foo: 2 })
+    expect(err2.info).to.eql({ foo: 2 })
+  })
+
+  it('can have a cause', function () {
+    const err1 = new OError('foo', { foo: 1 }, new Error('cause 1'))
+    expect(err1.cause.message).to.equal('cause 1')
+
+    const err2 = new OError('foo').withCause(new Error('cause 2'))
+    expect(err2.cause.message).to.equal('cause 2')
+  })
+
   it('handles a custom error type with a cause', function () {
     function doSomethingBadInternally() {
       throw new Error('internal error')
@@ -24,27 +43,27 @@ describe('OError', function () {
     function doSomethingBad() {
       try {
         doSomethingBadInternally()
-      } catch (err) {
-        throw new CustomError1({ userId: 123 }).withCause(err)
+      } catch (error) {
+        throw new CustomError1().withCause(error)
       }
     }
 
     try {
       doSomethingBad()
       expect.fail('should have thrown')
-    } catch (e) {
-      expectError(e, {
+    } catch (error) {
+      expectError(error, {
         name: 'CustomError1',
         klass: CustomError1,
-        message: 'CustomError1: failed to foo: internal error',
+        message: 'CustomError1: failed to foo',
         firstFrameRx: /doSomethingBad/,
       })
-      expect(OError.getFullInfo(e)).to.deep.equal({ userId: 123 })
-      const fullStack = OError.getFullStack(e)
-      expect(fullStack).to.match(
-        /^CustomError1: failed to foo: internal error$/m
-      )
-      expect(fullStack).to.match(/^caused by: Error: internal error$/m)
+      expect(OError.getFullInfo(error)).to.deep.equal({})
+      expectFullStackWithoutStackFramesToEqual(error, [
+        'CustomError1: failed to foo',
+        'caused by:',
+        '    Error: internal error',
+      ])
     }
   })
 
@@ -56,51 +75,37 @@ describe('OError', function () {
     function doBar() {
       try {
         doSomethingBadInternally()
-      } catch (err) {
-        throw new CustomError2('failed to bar!', { inner: 'a' }).withCause(err)
+      } catch (error) {
+        throw new CustomError2('failed to bar!').withCause(error)
       }
     }
 
     function doFoo() {
       try {
         doBar()
-      } catch (err) {
-        throw new CustomError1({ userId: 123 }).withCause(err)
+      } catch (error) {
+        throw new CustomError1().withCause(error)
       }
     }
 
     try {
       doFoo()
       expect.fail('should have thrown')
-    } catch (e) {
-      expectError(e, {
+    } catch (error) {
+      expectError(error, {
         name: 'CustomError1',
         klass: CustomError1,
-        message: 'CustomError1: failed to foo: failed to bar!: internal error',
+        message: 'CustomError1: failed to foo',
         firstFrameRx: /doFoo/,
       })
-      expect(OError.getFullInfo(e)).to.deep.equal({
-        userId: 123,
-        inner: 'a',
-      })
-      const fullStack = OError.getFullStack(e)
-      expect(fullStack).to.match(
-        /^CustomError1: failed to foo: failed to bar!: internal error$/m
-      )
-      expect(fullStack).to.match(
-        /^caused by: CustomError2: failed to bar!: internal error$/m
-      )
-      expect(fullStack).to.match(/^caused by: Error: internal error$/m)
-    }
-  })
-
-  it('handles a custom error without info', function () {
-    try {
-      throw new CustomError1()
-    } catch (e) {
-      expect(OError.getFullInfo(e)).to.deep.equal({})
-      const infoKey = Object.keys(e).find((k) => k === 'info')
-      expect(infoKey).to.not.exist
+      expectFullStackWithoutStackFramesToEqual(error, [
+        'CustomError1: failed to foo',
+        'caused by:',
+        '    CustomError2: failed to bar!',
+        '    caused by:',
+        '        Error: internal error',
+      ])
+      expect(OError.getFullInfo(error)).to.deep.equal({})
     }
   })
 })

libraries/o-error/test/support/index.js

@@ -27,3 +27,12 @@ exports.expectError = function OErrorExpectError(e, expected) {
   // first stack frame should be the function where the error was thrown
   expect(e.stack.split('\n')[1]).to.match(expected.firstFrameRx)
 }
+
+exports.expectFullStackWithoutStackFramesToEqual = function (error, expected) {
+  // But the stack contains all of the errors and tags.
+  const fullStack = OError.getFullStack(error)
+  const fullStackWithoutFrames = fullStack
+    .split('\n')
+    .filter((line) => !/^\s+at\s/.test(line))
+  expect(fullStackWithoutFrames).to.deep.equal(expected)
+}