From 68a5fafd1b70b4dea3ad30f305e2a31936cbc768 Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 31 Mar 2017 00:13:17 +0200 Subject: [PATCH 1/6] Updated ready/after to accept errors --- boot.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/boot.js b/boot.js index 32244bd..3249546 100644 --- a/boot.js +++ b/boot.js @@ -66,6 +66,7 @@ function Boot (server, opts, done) { this._server = server this._current = [] + this._error = null if (done) { this.once('start', done) @@ -93,7 +94,10 @@ Boot.prototype._init = function () { }, {}, noop) loadPlugin.call(this, main, (err) => { if (err) { - this.emit('error', err) + this._error = err + if (this._readyQ.length() === 0) { + throw err + } } else if (this._readyQ.length() > 0) { this._readyQ.resume() } else { @@ -146,7 +150,7 @@ Boot.prototype._addPlugin = function (plugin, opts, callback) { // we add the plugin to be loaded at the end of the current queue current.q.push(obj, (err) => { if (err) { - this.emit('error', err) + this._error = err } }) } @@ -222,14 +226,16 @@ function callWithCbOrNextTick (func, cb, context) { context = this._server } - if (func.length === 0) { - func() + if (func.length === 0 || func.length === 1) { + func(this._error) process.nextTick(cb) - } else if (func.length === 1) { - func(cb) + } else if (func.length === 2) { + func(this._error, cb) } else { - func(context, cb) + func(this._error, context, cb) } + // with this the error will appear just in the next after/ready callback + this._error = null } module.exports = Boot From c9332ef8fa6b93c5839df7097ca70f78b91e734a Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 31 Mar 2017 00:13:30 +0200 Subject: [PATCH 2/6] Updated test --- test/after-and-ready.js | 153 ++++++++++++++++++++++++++++++++++++---- test/chainable.js | 10 +-- 2 files changed, 144 insertions(+), 19 deletions(-) diff --git a/test/after-and-ready.js b/test/after-and-ready.js index 6f5c98d..d7db059 100644 --- a/test/after-and-ready.js +++ b/test/after-and-ready.js @@ -4,7 +4,7 @@ const test = require('tap').test const boot = require('..') test('boot a plugin and then execute a call after that', (t) => { - t.plan(4) + t.plan(5) const app = boot() let pluginLoaded = false @@ -16,7 +16,8 @@ test('boot a plugin and then execute a call after that', (t) => { done() }) - app.after(function (cb) { + app.after(function (err, cb) { + t.error(err) t.ok(pluginLoaded, 'afterred!') afterCalled = true cb() @@ -29,7 +30,7 @@ test('boot a plugin and then execute a call after that', (t) => { }) test('after without a done callback', (t) => { - t.plan(4) + t.plan(5) const app = boot() let pluginLoaded = false @@ -41,7 +42,8 @@ test('after without a done callback', (t) => { done() }) - app.after(function () { + app.after(function (err) { + t.error(err) t.ok(pluginLoaded, 'afterred!') afterCalled = true }) @@ -53,7 +55,7 @@ test('after without a done callback', (t) => { }) test('verify when a afterred call happens', (t) => { - t.plan(2) + t.plan(3) const app = boot() @@ -61,7 +63,8 @@ test('verify when a afterred call happens', (t) => { done() }) - app.after(function (cb) { + app.after(function (err, cb) { + t.error(err) cb() }, function () { t.pass('afterred finished') @@ -73,7 +76,7 @@ test('verify when a afterred call happens', (t) => { }) test('internal after', (t) => { - t.plan(17) + t.plan(18) const app = boot() let firstLoaded = false @@ -90,7 +93,8 @@ test('internal after', (t) => { t.notOk(thirdLoaded, 'third is not loaded') firstLoaded = true s.use(second) - s.after(function (cb) { + s.after(function (err, cb) { + t.error(err) t.notOk(afterCalled, 'after was not called') afterCalled = true cb() @@ -126,14 +130,15 @@ test('internal after', (t) => { }) test('ready adds at the end of the queue', (t) => { - t.plan(11) + t.plan(14) const app = boot() let pluginLoaded = false let afterCalled = false let readyCalled = false - app.ready(function (cb) { + app.ready(function (err, cb) { + t.error(err) t.ok(pluginLoaded, 'after the plugin') t.ok(afterCalled, 'after after') readyCalled = true @@ -145,7 +150,8 @@ test('ready adds at the end of the queue', (t) => { t.notOk(readyCalled, 'ready not called') pluginLoaded = true - app.ready(function () { + app.ready(function (err) { + t.error(err) t.ok(readyCalled, 'after the first ready') t.ok(afterCalled, 'after the after callback') }) @@ -153,7 +159,8 @@ test('ready adds at the end of the queue', (t) => { done() }) - app.after(function (cb) { + app.after(function (err, cb) { + t.error(err) t.ok(pluginLoaded, 'executing after!') t.notOk(readyCalled, 'ready not called') afterCalled = true @@ -168,7 +175,7 @@ test('ready adds at the end of the queue', (t) => { }) test('if the after/ready callback has two parameters, the first one must be the context', (t) => { - t.plan(2) + t.plan(4) const server = { my: 'server' } const app = boot(server) @@ -177,13 +184,129 @@ test('if the after/ready callback has two parameters, the first one must be the done() }) - app.after(function (context, cb) { + app.after(function (err, context, cb) { + t.error(err) t.equal(server, context) cb() }) - app.ready(function (context, cb) { + app.ready(function (err, context, cb) { + t.error(err) t.equal(server, context) cb() }) }) + +test('error should come in the first after - one parameter', (t) => { + t.plan(3) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }) + + app.after(function (err) { + t.ok(err instanceof Error) + t.is(err.message, 'err') + }) + + app.ready(function (err) { + t.error(err) + }) +}) + +test('error should come in the first after - two parameters', (t) => { + t.plan(3) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }) + + app.after(function (err, cb) { + t.ok(err instanceof Error) + t.is(err.message, 'err') + cb() + }) + + app.ready(function (err) { + t.error(err) + }) +}) + +test('error should come in the first after - three parameter', (t) => { + t.plan(4) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }) + + app.after(function (err, context, cb) { + t.ok(err instanceof Error) + t.is(err.message, 'err') + t.equal(context, server) + cb() + }) + + app.ready(function (err) { + t.error(err) + }) +}) + +test('error should come in the first ready - one parameter', (t) => { + t.plan(2) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }) + + app.ready(function (err) { + t.ok(err instanceof Error) + t.is(err.message, 'err') + }) +}) + +test('error should come in the first ready - two parameters', (t) => { + t.plan(2) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }) + + app.ready(function (err, cb) { + t.ok(err instanceof Error) + t.is(err.message, 'err') + cb() + }) +}) + +test('error should come in the first ready - three parameters', (t) => { + t.plan(3) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }) + + app.ready(function (err, context, cb) { + t.ok(err instanceof Error) + t.is(err.message, 'err') + t.equal(context, server) + cb() + }) +}) diff --git a/test/chainable.js b/test/chainable.js index 15c65fd..2d8e627 100644 --- a/test/chainable.js +++ b/test/chainable.js @@ -4,13 +4,14 @@ const test = require('tap').test const boot = require('..') test('chainable standalone', (t) => { - t.plan(4) + t.plan(5) boot() .use(function (ctx, opts, done) { t.pass('1st plugin') done() - }).after(function (done) { + }).after(function (err, done) { + t.error(err) t.pass('2nd after') done() }).ready(function () { @@ -22,7 +23,7 @@ test('chainable standalone', (t) => { }) test('chainable automatically binded', (t) => { - t.plan(4) + t.plan(5) const app = {} boot(app) @@ -31,7 +32,8 @@ test('chainable automatically binded', (t) => { .use(function (ctx, opts, done) { t.pass('1st plugin') done() - }).after(function (done) { + }).after(function (err, done) { + t.error(err) t.pass('2nd after') done() }).ready(function () { From 6611ca4571fe603571255d1f06d5620f33703a10 Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 31 Mar 2017 00:13:39 +0200 Subject: [PATCH 3/6] Updated docs --- README.md | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 13b6325..2de81cb 100644 --- a/README.md +++ b/README.md @@ -148,23 +148,24 @@ The functions will be loaded in the same order as they are inside the array. ------------------------------------------------------- -### app.after(func([context], [done]), [cb]) +### app.after(func(error, [context], [done]), [cb]) Calls a function after all the previously defined plugins are loaded, including all their dependencies. The `'start'` event is not emitted yet. -If one parameter is given to the callback, that parameter will be the `done` callback. -If two parameters are given to the callback, the first will be the `contenxt`, the second will be the `done` callback. +1. If one parameter is given to the callback, that parameter will be the `error` object. +2. If two parameters are given to the callback, the first will be the `error` object, the second will be the `done` callback. +3. If three parameters are given to the callback, the first will be the `error` object, the second will be the `context` and the third the `done` callback. ```js const server = {} ... // after with one parameter -boot.after(function (done) { +boot.after(function (err, done) { done() }) // after with two parameters -boot.after(function (context, done) { +boot.after(function (err, context, done) { assert.equal(context, server) done() }) @@ -178,23 +179,25 @@ chainable API. ------------------------------------------------------- -### app.ready(func([context], [done])) +### app.ready(func(error, [context], [done])) Calls a functon after all the plugins and `after` call are completed, but befer `'start'` is emitted. `ready` callbacks are executed one at a time. -If one parameter is given to the callback, that parameter will be the `done` callback. -If two parameters are given to the callback, the first will be the `contenxt`, the second will be the `done` callback. +1. If one parameter is given to the callback, that parameter will be the `error` object. +2. If two parameters are given to the callback, the first will be the `error` object, the second will be the `done` callback. +3. If three parameters are given to the callback, the first will be the `error` object, the second will be the `context` and the third the `done` callback. + ```js const server = {} ... // ready with one parameter -boot.ready(function (done) { +boot.ready(function (err, done) { done() }) // ready with two parameters -boot.ready(function (context, done) { +boot.ready(function (err, context, done) { assert.equal(context, server) done() }) From a0c67bb937299d1487a5c653557c2af2a4741d16 Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 31 Mar 2017 14:43:29 +0200 Subject: [PATCH 4/6] Updated docs --- README.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 2de81cb..2ca47e5 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ const boot = require('avvio')() boot .use(first, { hello: 'world' }) - .after((cb) => { + .after((err, cb) => { console.log('after first and second') cb() }) @@ -103,7 +103,7 @@ server.use(function first (s, opts, cb) { s.use(function second (s, opts, cb) { cb() }, cb) -}).after(function (cb) { +}).after(function (err, cb) { // after first and second are finished cb() }) @@ -115,7 +115,6 @@ Options: Events: -* `'error'`  if something bad happens * `'start'`  when the application starts The `boot` function can be used also as a @@ -159,12 +158,12 @@ all their dependencies. The `'start'` event is not emitted yet. ```js const server = {} ... -// after with one parameter +// after with two parameter boot.after(function (err, done) { done() }) -// after with two parameters +// after with three parameters boot.after(function (err, context, done) { assert.equal(context, server) done() @@ -191,12 +190,12 @@ executed one at a time. ```js const server = {} ... -// ready with one parameter +// ready with two parameter boot.ready(function (err, done) { done() }) -// ready with two parameters +// ready with three parameters boot.ready(function (err, context, done) { assert.equal(context, server) done() From b5ccb3e93b3705e7c6c89c1f6d931e7d9883bcc5 Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 31 Mar 2017 14:43:56 +0200 Subject: [PATCH 5/6] Updated error handling --- boot.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/boot.js b/boot.js index 3249546..a78baa4 100644 --- a/boot.js +++ b/boot.js @@ -137,7 +137,7 @@ Boot.prototype._addPlugin = function (plugin, opts, callback) { throw new Error('plugin must be a function') } opts = opts || {} - callback = callback || noop + callback = callback || null // we reinit, if use is called after emitting start once this._init() @@ -197,8 +197,14 @@ Plugin.prototype.exec = function (server, cb) { Plugin.prototype.finish = function (err, cb) { const callback = this.callback - callback(err) - cb(err) + // if 'use' has a callback + if (callback) { + callback(err) + // if 'use' has a callback but does not have parameters + cb(callback.length > 0 ? null : err) + } else { + cb(err) + } } // loads a plugin From 5af6c6984a667ac5bb415a6e36606265801a5b88 Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 31 Mar 2017 14:44:05 +0200 Subject: [PATCH 6/6] Updated test --- test/after-and-ready.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/after-and-ready.js b/test/after-and-ready.js index d7db059..214053b 100644 --- a/test/after-and-ready.js +++ b/test/after-and-ready.js @@ -310,3 +310,35 @@ test('error should come in the first ready - three parameters', (t) => { cb() }) }) + +test('if `use` has a callback with more then one parameter, the error must not reach ready', (t) => { + t.plan(2) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }, err => { + t.ok(err) + }) + + app.ready(function (err) { + t.error(err) + }) +}) + +test('if `use` has a callback without parameters, the error must reach ready', (t) => { + t.plan(1) + + const server = { my: 'server' } + const app = boot(server) + + app.use(function (s, opts, done) { + done(new Error('err')) + }, () => {}) + + app.ready(function (err) { + t.ok(err) + }) +})