Skip to content

Commit

Permalink
Do not crash if nested plugin does not call next (#252)
Browse files Browse the repository at this point in the history
* Do not crash if nested plugin does not call next

Signed-off-by: Matteo Collina <hello@matteocollina.com>

* fixup

Signed-off-by: Matteo Collina <hello@matteocollina.com>

---------

Signed-off-by: Matteo Collina <hello@matteocollina.com>
  • Loading branch information
mcollina authored Apr 19, 2024
1 parent 7207f16 commit c43f269
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
10 changes: 10 additions & 0 deletions boot.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ Boot.prototype._addPlugin = function (pluginFn, opts, isAfter) {
// we always add plugins to load at the current element
const current = this._current[0]

// In case of promises, adjust the timeout
if (isAfter && this._lastUsed) {
// We need to decrease it by 2ms to make sure the internal timeout
// is triggered earlier
const delta = Date.now() - current.startTime + 2
if (this._lastUsed.timeout > 0 && delta > 0) {
this._lastUsed.timeout = this._lastUsed.timeout - delta
}
}

const plugin = new Plugin(fastq(this, this._loadPluginNextTick, 1), pluginFn, opts, isAfter, this._opts.timeout)
this._trackPluginLoading(plugin)

Expand Down
19 changes: 12 additions & 7 deletions lib/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ function Plugin (queue, func, options, isAfter, timeout) {
this.loaded = false

this._promise = null

this.startTime = null
}

inherits(Plugin, EventEmitter)
Expand Down Expand Up @@ -117,6 +119,7 @@ Plugin.prototype.exec = function (server, callback) {
}

this.started = true
this.startTime = Date.now()
this.emit('start', this.server ? this.server.name : null, this.name, Date.now())

const maybePromiseLike = func(this.server, this.options, done)
Expand Down Expand Up @@ -145,14 +148,16 @@ Plugin.prototype.loadedSoFar = function () {
this._error = afterErr
this.queue.pause()

if (afterErr) {
debug('rejecting promise', this.name, afterErr)
this._promise.reject(afterErr)
} else {
debug('resolving promise', this.name)
this._promise.resolve()
if (this._promise) {
if (afterErr) {
debug('rejecting promise', this.name, afterErr)
this._promise.reject(afterErr)
} else {
debug('resolving promise', this.name)
this._promise.resolve()
}
this._promise = null
}
this._promise = null

process.nextTick(callback, afterErr)
})
Expand Down
21 changes: 21 additions & 0 deletions test/plugin-timeout.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,24 @@ test('timeout without calling next in ready and rethrowing the error', (t) => {

app.start()
})

test('nested timeout do not crash - await', (t) => {
t.plan(4)
const app = boot({}, {
timeout: 10 // 10 ms
})
app.use(one)
async function one (app, opts) {
await app.use(two)
}

function two (app, opts, next) {
// do not call next on purpose
}
app.ready((err) => {
t.ok(err)
t.equal(err.fn, two)
t.equal(err.message, message('two'))
t.equal(err.code, 'AVV_ERR_PLUGIN_EXEC_TIMEOUT')
})
})

0 comments on commit c43f269

Please sign in to comment.