Skip to content

Commit

Permalink
Reject promises in cases of error when await app.register() or .after()
Browse files Browse the repository at this point in the history
  • Loading branch information
mcollina committed Feb 17, 2020
1 parent ca09364 commit 766f542
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 26 deletions.
20 changes: 15 additions & 5 deletions boot.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,17 @@ Boot.prototype.use = function (plugin, opts) {
return this
}

Boot.prototype._loadRegistered = function (plugin) {
plugin = plugin || this._lastUsed
return new Promise((resolve) => {
Boot.prototype._loadRegistered = function () {
const plugin = this._current[0]
return new Promise((resolve, reject) => {
var weNeedToStart = !this.started && !this.booted
if (plugin && !plugin.loaded) {
plugin.asyncQ.push(() => {
debug('_loadRegistered deferring promise', plugin.name)
plugin.pushToAsyncQ((err) => {
if (err) {
reject(err)
return
}
resolve()
})
} else {
Expand All @@ -211,7 +217,7 @@ Boot.prototype._loadRegistered = function (plugin) {

// if the root plugin is not loaded, let's resume that
// so one can use after() befor calling ready
if (!this.started && !this.booted) {
if (weNeedToStart) {
this._root.q.resume()
}
})
Expand Down Expand Up @@ -253,6 +259,10 @@ Boot.prototype._addPlugin = function (plugin, opts, isAfter) {
}

Boot.prototype.after = function (func) {
if (!func) {
return this._loadRegistered()
}

this._addPlugin(_after.bind(this), {}, true)

function _after (s, opts, done) {
Expand Down
56 changes: 36 additions & 20 deletions plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,19 @@ function getName (func) {
}

function Plugin (parent, func, optsOrFunc, isAfter, timeout) {
this.started = false
this.func = func
this.opts = optsOrFunc
this.deferred = false
this.onFinish = null
this.parent = parent
this.timeout = timeout === undefined ? parent._timeout : timeout
this.name = getName(func)
this.isAfter = isAfter
this.q = fastq(parent, loadPlugin, 1)
this.q.pause()
this.asyncQ = fastq(parent, (resolve, cb) => {
resolve(this.server)
this._error = null
this.asyncQ = fastq(parent, (done, cb) => {
done(this._error || this.parent._error)
cb()
}, 1)
this.asyncQ.pause()
Expand Down Expand Up @@ -83,12 +84,10 @@ Plugin.prototype.exec = function (server, cb) {
return
}

this._error = err

if (err) {
debug('exec errored', name)

// In case of errors, we need to kickstart
// the asyncQ as it won't get started otherwise
this.asyncQ.resume()
} else {
debug('exec completed', name)
}
Expand All @@ -114,19 +113,12 @@ Plugin.prototype.exec = function (server, cb) {
}, this.timeout)
}

this.started = true
this.emit('start', this.server ? this.server.name : null, this.name, Date.now())
var promise = func(this.server, this.opts, done)

if (promise && typeof promise.then === 'function') {
debug('resolving promise', name)
queueMicrotask(() => {
if (this.asyncQ.length() > 0) {
this.server.after(() => {
this.asyncQ.resume()
})
}
this.q.resume()
})

promise.then(
() => process.nextTick(done),
Expand All @@ -140,8 +132,31 @@ Plugin.prototype.enqueue = function (obj, cb) {
this.q.push(obj, cb)
}

Plugin.prototype.pushToAsyncQ = function (fn) {
if (!this.server) {
this.on('start', () => this._pushToAsyncQ(fn))
return
}

this._pushToAsyncQ(fn)
}

Plugin.prototype._pushToAsyncQ = function (fn) {
debug('_pushToAsyncQ', this.name)
if (this.asyncQ.length() === 0) {
this.server.after((err, cb) => {
this.q.pause()
debug('resuming asyncQ', this.name)
this.asyncQ.resume()
process.nextTick(cb, err)
})
}
this.asyncQ.push(fn)
this.q.resume()
}

Plugin.prototype.finish = function (err, cb) {
debug('finish', this.name)
debug('finish', this.name, err)
const done = () => {
if (this.loaded) {
return
Expand All @@ -155,18 +170,19 @@ Plugin.prototype.finish = function (err, cb) {
}

if (err) {
this.asyncQ.resume()
done()
return
}

const check = () => {
debug('check', this.name, this.q.length(), this.q.running())
debug('check', this.name, this.q.length(), this.q.running(), this.asyncQ.length())
if (this.q.length() === 0 && this.q.running() === 0) {
if (this.asyncQ.length() > 0) {
this.asyncQ.drain = () => {
this.asyncQ.drain = noop
this.asyncQ.pause()
check()
queueMicrotask(check)
}
this.asyncQ.resume()
} else {
Expand All @@ -181,12 +197,12 @@ Plugin.prototype.finish = function (err, cb) {

// we defer the check, as a safety net for things
// that might be scheduled in the loading callback
process.nextTick(check)
queueMicrotask(check)
}
}
}

process.nextTick(check)
queueMicrotask(check)

// we start loading the dependents plugins only once
// the current level is finished
Expand Down
36 changes: 35 additions & 1 deletion test/await-after.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,27 @@ test('await after - nested plugins with same tick callbacks', async (t) => {
t.pass('reachable')
})

test('await after without server', async (t) => {
const app = boot()

let secondLoaded = false

app.use(async (app) => {
t.pass('plugin init')
app.use(async () => {
t.pass('plugin2 init')
await sleep(1)
secondLoaded = true
})
})
await app.after()
t.pass('reachable')
t.is(secondLoaded, true)

await app.ready()
t.pass('reachable')
})

test('await after - nested plugins with future tick callbacks', async (t) => {
const app = {}
boot(app)
Expand Down Expand Up @@ -202,7 +223,7 @@ test('await after - error handling, future tick cb err, nested', async (t) => {
t.rejects(() => app.ready(), Error('kaboom'))
})

test('await after complex scenario', async (t) => {
test('await after complex scenario', { only: true }, async (t) => {
const app = {}
boot(app)
t.plan(16)
Expand Down Expand Up @@ -309,3 +330,16 @@ test('without autostart and with override', async (t) => {

await app.ready()
})

test('stop processing after errors', async (t) => {
const app = boot()

try {
await app.use(async function first (app) {
t.pass('first should be loaded')
throw new Error('kaboom')
})
} catch (e) {
t.is(e.message, 'kaboom')
}
})

0 comments on commit 766f542

Please sign in to comment.