diff --git a/lib/application.js b/lib/application.js index d44435afc..ee7f89e03 100644 --- a/lib/application.js +++ b/lib/application.js @@ -195,7 +195,13 @@ module.exports = class Application extends Emitter { */ onerror(err) { - if (!(err instanceof Error)) throw new TypeError(util.format('non-error thrown: %j', err)); + // When dealing with cross-globals a normal `instanceof` check doesn't work properly. + // See https://github.com/koajs/koa/issues/1466 + // We can probably remove it once jest fixes https://github.com/facebook/jest/issues/2549. + const isNativeError = + Object.prototype.toString.call(err) === '[object Error]' || + err instanceof Error; + if (!isNativeError) throw new TypeError(util.format('non-error thrown: %j', err)); if (404 === err.status || err.expose) return; if (this.silent) return; diff --git a/lib/context.js b/lib/context.js index 894fa131d..f6c0f111b 100644 --- a/lib/context.js +++ b/lib/context.js @@ -110,7 +110,13 @@ const proto = module.exports = { // to node-style callbacks. if (null == err) return; - if (!(err instanceof Error)) err = new Error(util.format('non-error thrown: %j', err)); + // When dealing with cross-globals a normal `instanceof` check doesn't work properly. + // See https://github.com/koajs/koa/issues/1466 + // We can probably remove it once jest fixes https://github.com/facebook/jest/issues/2549. + const isNativeError = + Object.prototype.toString.call(err) === '[object Error]' || + err instanceof Error; + if (!isNativeError) err = new Error(util.format('non-error thrown: %j', err)); let headerSent = false; if (this.headerSent || !this.writable) { diff --git a/test/application/onerror.js b/test/application/onerror.js index 487360bd1..624e98899 100644 --- a/test/application/onerror.js +++ b/test/application/onerror.js @@ -16,6 +16,18 @@ describe('app.onerror(err)', () => { }, TypeError, 'non-error thrown: foo'); }); + it('should accept errors coming from other scopes', () => { + const ExternError = require('vm').runInNewContext('Error'); + + const app = new Koa(); + const error = Object.assign(new ExternError('boom'), { + status: 418, + expose: true + }); + + assert.doesNotThrow(() => app.onerror(error)); + }); + it('should do nothing if status is 404', () => { const app = new Koa(); const err = new Error(); diff --git a/test/context/onerror.js b/test/context/onerror.js index 4e59b5f3d..2c03e27bd 100644 --- a/test/context/onerror.js +++ b/test/context/onerror.js @@ -1,4 +1,3 @@ - 'use strict'; const assert = require('assert'); @@ -205,6 +204,40 @@ describe('ctx.onerror(err)', () => { }); }); + describe('when error from another scope thrown', () => { + it('should handle it like a normal error', async() => { + const ExternError = require('vm').runInNewContext('Error'); + + const app = new Koa(); + const error = Object.assign(new ExternError('boom'), { + status: 418, + expose: true + }); + app.use((ctx, next) => { + throw error; + }); + + const server = app.listen(); + + const gotRightErrorPromise = new Promise((resolve, reject) => { + app.on('error', receivedError => { + try { + assert.strictEqual(receivedError, error); + resolve(); + } catch (e) { + reject(e); + } + }); + }); + + await request(server) + .get('/') + .expect(418); + + await gotRightErrorPromise; + }); + }); + describe('when non-error thrown', () => { it('should response non-error thrown message', () => { const app = new Koa(); @@ -248,7 +281,7 @@ describe('ctx.onerror(err)', () => { }); app.use(async ctx => { - throw {key: 'value'}; // eslint-disable-line no-throw-literal + throw { key: 'value' }; // eslint-disable-line no-throw-literal }); request(app.callback())