From c43f2698298ad356d518e8df73e4f5321cf1ab75 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 19 Apr 2024 11:01:49 +0200 Subject: [PATCH] Do not crash if nested plugin does not call next (#252) * Do not crash if nested plugin does not call next Signed-off-by: Matteo Collina * fixup Signed-off-by: Matteo Collina --------- Signed-off-by: Matteo Collina --- boot.js | 10 ++++++++++ lib/plugin.js | 19 ++++++++++++------- test/plugin-timeout.test.js | 21 +++++++++++++++++++++ 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/boot.js b/boot.js index beac001..db5edab 100644 --- a/boot.js +++ b/boot.js @@ -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) diff --git a/lib/plugin.js b/lib/plugin.js index 2046636..8c62d87 100644 --- a/lib/plugin.js +++ b/lib/plugin.js @@ -50,6 +50,8 @@ function Plugin (queue, func, options, isAfter, timeout) { this.loaded = false this._promise = null + + this.startTime = null } inherits(Plugin, EventEmitter) @@ -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) @@ -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) }) diff --git a/test/plugin-timeout.test.js b/test/plugin-timeout.test.js index 7fdd48c..22b97ca 100644 --- a/test/plugin-timeout.test.js +++ b/test/plugin-timeout.test.js @@ -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') + }) +})