mirrors/etherpad-lite
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2025-04-22 08:26:16 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 26

hooks: Rewrite callAll and aCallAll for consistency

Browse source 
Rewrite the `callAll` and `aCallAll` functions to support all
reasonable hook behaviors and to report errors for unreasonable
behaviors (e.g., calling the callback twice).

Now a hook function like the following works as expected when invoked
by `aCallAll`:

```
exports.myHookFn = (hookName, context, cb) => {
  cb('some value');
  return;
};
```
This commit is contained in:
Richard Hansen 2020-10-08 01:42:49 -04:00 • committed by John McLear
parent 79119baf58
commit 36aceb3aba
6 changed files with 1400 additions and 74 deletions
Download patch file Download diff file Expand all files Collapse all files

113
src/package-lock.json generated
View file

@ -454,6 +454,51 @@
"resolved": "https://registry.npmjs.org/@kwsites/promise-deferred/-/promise-deferred-1.1.1.tgz",
"integrity": "sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw=="
},
"@sinonjs/commons": {
"version": "1.8.1",
"resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.8.1.tgz",
"integrity": "sha512-892K+kWUUi3cl+LlqEWIDrhvLgdL79tECi8JZUyq6IviKy/DNhuzCRlbHUjxK89f4ypPMMaFnFuR9Ie6DoIMsw==",
"dev": true,
"requires": {
"type-detect": "4.0.8"
}
},
"@sinonjs/fake-timers": {
"version": "6.0.1",
"resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-6.0.1.tgz",
"integrity": "sha512-MZPUxrmFubI36XS1DI3qmI0YdN1gks62JtFZvxR67ljjSNCeK6U08Zx4msEWOXuofgqUt6zPHSi1H9fbjR/NRA==",
"dev": true,
"requires": {
"@sinonjs/commons": "^1.7.0"
}
},
"@sinonjs/formatio": {
"version": "5.0.1",
"resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-5.0.1.tgz",
"integrity": "sha512-KaiQ5pBf1MpS09MuA0kp6KBQt2JUOQycqVG1NZXvzeaXe5LGFqAKueIS0bw4w0P9r7KuBSVdUk5QjXsUdu2CxQ==",
"dev": true,
"requires": {
"@sinonjs/commons": "^1",
"@sinonjs/samsam": "^5.0.2"
}
},
"@sinonjs/samsam": {
"version": "5.2.0",
"resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-5.2.0.tgz",
"integrity": "sha512-CaIcyX5cDsjcW/ab7HposFWzV1kC++4HNsfnEdFJa7cP1QIuILAKV+BgfeqRXhcnSAc76r/Rh/O5C+300BwUIw==",
"dev": true,
"requires": {
"@sinonjs/commons": "^1.6.0",
"lodash.get": "^4.4.2",
"type-detect": "^4.0.8"
}
},
"@sinonjs/text-encoding": {
"version": "0.7.1",
"resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz",
"integrity": "sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==",
"dev": true
},
"@types/caseless": {
"version": "0.12.2",
"resolved": "https://registry.npmjs.org/@types/caseless/-/caseless-0.12.2.tgz",
@ -2742,6 +2787,12 @@
"verror": "1.10.0"
}
},
"just-extend": {
"version": "4.1.1",
"resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.1.1.tgz",
"integrity": "sha512-aWgeGFW67BP3e5181Ep1Fv2v8z//iBJfrvyTnq8wG86vEESwmonn1zPBJ0VfmT9CJq2FIT0VsETtrNFm2a+SHA==",
"dev": true
},
"jwa": {
"version": "1.4.1",
"resolved": "https://registry.npmjs.org/jwa/-/jwa-1.4.1.tgz",
@ -3269,6 +3320,30 @@
"resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.6.2.tgz",
"integrity": "sha512-hZXc7K2e+PgeI1eDBe/10Ard4ekbfrrqG8Ep+8Jmf4JID2bNg7NvCPOZN+kfF574pFQI7mum2AUqDidoKqcTOw=="
},
"nise": {
"version": "4.0.4",
"resolved": "https://registry.npmjs.org/nise/-/nise-4.0.4.tgz",
"integrity": "sha512-bTTRUNlemx6deJa+ZyoCUTRvH3liK5+N6VQZ4NIw90AgDXY6iPnsqplNFf6STcj+ePk0H/xqxnP75Lr0J0Fq3A==",
"dev": true,
"requires": {
"@sinonjs/commons": "^1.7.0",
"@sinonjs/fake-timers": "^6.0.0",
"@sinonjs/text-encoding": "^0.7.1",
"just-extend": "^4.0.2",
"path-to-regexp": "^1.7.0"
},
"dependencies": {
"path-to-regexp": {
"version": "1.8.0",
"resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.8.0.tgz",
"integrity": "sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==",
"dev": true,
"requires": {
"isarray": "0.0.1"
}
}
}
},
"node-environment-flags": {
"version": "1.0.6",
"resolved": "https://registry.npmjs.org/node-environment-flags/-/node-environment-flags-1.0.6.tgz",
@ -7396,6 +7471,44 @@
}
}
},
"sinon": {
"version": "9.2.0",
"resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.0.tgz",
"integrity": "sha512-eSNXz1XMcGEMHw08NJXSyTHIu6qTCOiN8x9ODACmZpNQpr0aXTBXBnI4xTzQzR+TEpOmLiKowGf9flCuKIzsbw==",
"dev": true,
"requires": {
"@sinonjs/commons": "^1.8.1",
"@sinonjs/fake-timers": "^6.0.1",
"@sinonjs/formatio": "^5.0.1",
"@sinonjs/samsam": "^5.2.0",
"diff": "^4.0.2",
"nise": "^4.0.4",
"supports-color": "^7.1.0"
},
"dependencies": {
"diff": {
"version": "4.0.2",
"resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz",
"integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==",
"dev": true
},
"has-flag": {
"version": "4.0.0",
"resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz",
"integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==",
"dev": true
},
"supports-color": {
"version": "7.2.0",
"resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz",
"integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==",
"dev": true,
"requires": {
"has-flag": "^4.0.0"
}
}
}
},
"slide": {
"version": "1.1.6",
"resolved": "https://registry.npmjs.org/slide/-/slide-1.1.6.tgz",

5
src/package.json
View file

@ -77,14 +77,15 @@
"etherpad-lite": "./node/server.js"
},
"devDependencies": {
"etherpad-cli-client": "0.0.9",
"mocha": "7.1.2",
"mocha-froth": "^0.2.10",
"nyc": "15.0.1",
"set-cookie-parser": "^2.4.6",
"sinon": "^9.2.0",
"superagent": "^3.8.3",
"supertest": "4.0.2",
"wd": "1.12.1",
"etherpad-cli-client": "0.0.9"
"wd": "1.12.1"
},
"engines": {
"node": ">=10.13.0",

322
src/static/js/pluginfw/hooks.js
View file

@ -1,3 +1,5 @@
/* global exports, require */
var _ = require("underscore");
var pluginDefs = require('./plugin_defs');
@ -17,8 +19,8 @@ function checkDeprecation(hook) {
const notice = exports.deprecationNotices[hook.hook_name];
if (notice == null) return;
if (deprecationWarned[hook.hook_fn_name]) return;
console.warn('%s hook used by the %s plugin (%s) is deprecated: %s',
hook.hook_name, hook.part.name, hook.hook_fn_name, notice);
console.warn(`${hook.hook_name} hook used by the ${hook.part.name} plugin ` +
`(${hook.hook_fn_name}) is deprecated: ${notice}`);
deprecationWarned[hook.hook_fn_name] = true;
}
@ -76,49 +78,287 @@ exports.mapFirst = function (lst, fn, cb, predicate) {
next();
}
exports.callAll = function (hook_name, args) {
if (!args) args = {};
if (pluginDefs.hooks[hook_name] === undefined) return [];
return _.flatten(_.map(pluginDefs.hooks[hook_name], function(hook) {
return hookCallWrapper(hook, hook_name, args);
}), true);
}
// Calls the hook function synchronously and returns the value provided by the hook function (via
// callback or return value).
//
// A synchronous hook function can provide a value in these ways:
//
// * Call the callback, passing the desired value (which may be `undefined`) directly as the first
// argument, then return `undefined`.
// * For hook functions with three (or more) parameters: Directly return the desired value, which
// must not be `undefined`. Note: If a three-parameter hook function directly returns
// `undefined` and it has not already called the callback then it is indicating that it is not
// yet done and will eventually call the callback. This behavior is not supported by synchronous
// hooks.
// * For hook functions with two (or fewer) parameters: Directly return the desired value (which
// may be `undefined`).
//
// The callback passed to a hook function is guaranteed to return `undefined`, so it is safe for
// hook functions to do `return cb(value);`.
//
// A hook function can signal an error by throwing.
//
// A hook function settles when it provides a value (via callback or return) or throws. If a hook
// function attempts to settle again (e.g., call the callback again, or call the callback and also
// return a value) then the second attempt has no effect except either an error message is logged or
// there will be an unhandled promise rejection depending on whether the the subsequent attempt is a
// duplicate (same value or error) or different, respectively.
//
// See the tests in tests/backend/specs/hooks.js for examples of supported and prohibited behaviors.
//
function callHookFnSync(hook, context) {
checkDeprecation(hook);
async function aCallAll(hook_name, args, cb) {
if (!args) args = {};
if (!cb) cb = function () {};
if (pluginDefs.hooks[hook_name] === undefined) return cb(null, []);
// This var is used to keep track of whether the hook function already settled.
let outcome;
var hooksPromises = pluginDefs.hooks[hook_name].map(async function(hook, index) {
return await hookCallWrapper(hook, hook_name, args, function (res) {
return Promise.resolve(res);
});
});
// This is used to prevent recursion.
let doubleSettleErr;
var result = await Promise.all(hooksPromises);
// after forEach
cb(null, _.flatten(result, true));
}
/* return a Promise if cb is not supplied */
exports.aCallAll = function (hook_name, args, cb) {
if (cb === undefined) {
try{
return new Promise(function(resolve, reject) {
aCallAll(hook_name, args, function(err, res) {
return err ? reject(err) : resolve(res);
});
});
}catch(e){
$.gritter.removeAll();
$.gritter.add("Please update your web browser")
const settle = (err, val, how) => {
doubleSettleErr = null;
const state = err == null ? 'resolved' : 'rejected';
if (outcome != null) {
// It was already settled, which indicates a bug.
const action = err == null ? 'resolve' : 'reject';
const msg = (`DOUBLE SETTLE BUG IN HOOK FUNCTION (plugin: ${hook.part.name}, ` +
`function name: ${hook.hook_fn_name}, hook: ${hook.hook_name}): ` +
`Attempt to ${action} via ${how} but it already ${outcome.state} ` +
`via ${outcome.how}. Ignoring this attempt to ${action}.`);
console.error(msg);
if (state !== outcome.state || (err == null ? val !== outcome.val : err !== outcome.err)) {
// The second settle attempt differs from the first, which might indicate a serious bug.
doubleSettleErr = new Error(msg);
throw doubleSettleErr;
}
return;
}
} else {
return aCallAll(hook_name, args, cb);
outcome = {state, err, val, how};
if (val && typeof val.then === 'function') {
console.error(`PROHIBITED PROMISE BUG IN HOOK FUNCTION (plugin: ${hook.part.name}, ` +
`function name: ${hook.hook_fn_name}, hook: ${hook.hook_name}): ` +
'The hook function provided a "thenable" (e.g., a Promise) which is ' +
'prohibited because the hook expects to get the value synchronously.');
}
};
// IMPORTANT: This callback must return `undefined` so that a hook function can safely do
// `return callback(value);` for backwards compatibility.
const callback = (ret) => {
settle(null, ret, 'callback');
};
let val;
try {
val = hook.hook_fn(hook.hook_name, context, callback);
} catch (err) {
if (err === doubleSettleErr) throw err; // Avoid recursion.
try {
settle(err, null, 'thrown exception');
} catch (doubleSettleErr) {
// Schedule the throw of the double settle error on the event loop via
// Promise.resolve().then() (which will result in an unhandled Promise rejection) so that the
// original error is the error that is seen by the caller. Fixing the original error will
// likely fix the double settle bug, so the original error should get priority.
Promise.resolve().then(() => { throw doubleSettleErr; });
}
throw err;
}
// IMPORTANT: This MUST check for undefined -- not nullish -- because some hooks intentionally use
// null as a special value.
if (val === undefined) {
if (outcome != null) return outcome.val; // Already settled via callback.
if (hook.hook_fn.length >= 3) {
console.error(`UNSETTLED FUNCTION BUG IN HOOK FUNCTION (plugin: ${hook.part.name}, ` +
`function name: ${hook.hook_fn_name}, hook: ${hook.hook_name}): ` +
'The hook function neither called the callback nor returned a non-undefined ' +
'value. This is prohibited because it will result in freezes when a future ' +
'version of Etherpad updates the hook to support asynchronous behavior.');
} else {
// The hook function is assumed to not have a callback parameter, so fall through and accept
// `undefined` as the resolved value.
//
// IMPORTANT: "Rest" parameters and default parameters are not counted in`Function.length`, so
// the assumption does not hold for wrappers like `(...args) => { real(...args); }`. Such
// functions will still work properly without any logged warnings or errors for now, but:
// * Once the hook is upgraded to support asynchronous hook functions, calling the callback
// will (eventually) cause a double settle error, and the function might prematurely
// resolve to `undefined` instead of the desired value.
// * The above "unsettled function" warning is not logged if the function fails to call the
// callback like it is supposed to.
}
}
settle(null, val, 'returned value');
return outcome.val;
}
// Invokes all registered hook functions synchronously.
//
// Arguments:
// * hookName: Name of the hook to invoke.
// * context: Passed unmodified to the hook functions, except nullish becomes {}.
//
// Return value:
// A flattened array of hook results. Specifically, it is equivalent to doing the following:
// 1. Collect all values returned by the hook functions into an array.
// 2. Convert each `undefined` entry into `[]`.
// 3. Flatten one level.
exports.callAll = function (hookName, context) {
if (context == null) context = {};
const hooks = pluginDefs.hooks[hookName] || [];
return _.flatten(hooks.map((hook) => {
const ret = callHookFnSync(hook, context);
// `undefined` (but not `null`!) is treated the same as [].
if (ret === undefined) return [];
return ret;
}), 1);
};
// Calls the hook function asynchronously and returns a Promise that either resolves to the hook
// function's provided value or rejects with an error generated by the hook function.
//
// An asynchronous hook function can provide a value in these ways:
//
// * Call the callback, passing a Promise (or thenable) that resolves to the desired value (which
// may be `undefined`) as the first argument.
// * Call the callback, passing the desired value (which may be `undefined`) directly as the first
// argument.
// * Return a Promise (or thenable) that resolves to the desired value (which may be `undefined`).
// * For hook functions with three (or more) parameters: Directly return the desired value, which
// must not be `undefined`. Note: If a hook function directly returns `undefined` and it has not
// already called the callback then it is indicating that it is not yet done and will eventually
// call the callback.
// * For hook functions with two (or fewer) parameters: Directly return the desired value (which
// may be `undefined`).
//
// The callback passed to a hook function is guaranteed to return `undefined`, so it is safe for
// hook functions to do `return cb(valueOrPromise);`.
//
// A hook function can signal an error in these ways:
//
// * Throw.
// * Return a Promise that rejects.
// * Pass a Promise that rejects as the first argument to the provided callback.
//
// A hook function settles when it directly provides a value, when it throws, or when the Promise it
// provides settles (resolves or rejects). If a hook function attempts to settle again (e.g., call
// the callback again, or return a value and also call the callback) then the second attempt has no
// effect except either an error message is logged or an Error object is thrown depending on whether
// the the subsequent attempt is a duplicate (same value or error) or different, respectively.
//
// See the tests in tests/backend/specs/hooks.js for examples of supported and prohibited behaviors.
//
async function callHookFnAsync(hook, context) {
checkDeprecation(hook);
return await new Promise((resolve, reject) => {
// This var is used to keep track of whether the hook function already settled.
let outcome;
const settle = (err, val, how) => {
const state = err == null ? 'resolved' : 'rejected';
if (outcome != null) {
// It was already settled, which indicates a bug.
const action = err == null ? 'resolve' : 'reject';
const msg = (`DOUBLE SETTLE BUG IN HOOK FUNCTION (plugin: ${hook.part.name}, ` +
`function name: ${hook.hook_fn_name}, hook: ${hook.hook_name}): ` +
`Attempt to ${action} via ${how} but it already ${outcome.state} ` +
`via ${outcome.how}. Ignoring this attempt to ${action}.`);
console.error(msg);
if (state !== outcome.state || (err == null ? val !== outcome.val : err !== outcome.err)) {
// The second settle attempt differs from the first, which might indicate a serious bug.
throw new Error(msg);
}
return;
}
outcome = {state, err, val, how};
if (err == null) { resolve(val); } else { reject(err); }
};
// IMPORTANT: This callback must return `undefined` so that a hook function can safely do
// `return callback(value);` for backwards compatibility.
const callback = (ret) => {
// Wrap ret in a Promise so that a hook function can do `callback(asyncFunction());`. Note: If
// ret is a Promise (or other thenable), Promise.resolve() will flatten it into this new
// Promise.
Promise.resolve(ret).then(
(val) => settle(null, val, 'callback'),
(err) => settle(err, null, 'rejected Promise passed to callback'));
};
let ret;
try {
ret = hook.hook_fn(hook.hook_name, context, callback);
} catch (err) {
try {
settle(err, null, 'thrown exception');
} catch (doubleSettleErr) {
// Schedule the throw of the double settle error on the event loop via
// Promise.resolve().then() (which will result in an unhandled Promise rejection) so that
// the original error is the error that is seen by the caller. Fixing the original error
// will likely fix the double settle bug, so the original error should get priority.
Promise.resolve().then(() => { throw doubleSettleErr; });
}
throw err;
}
// IMPORTANT: This MUST check for undefined -- not nullish -- because some hooks intentionally
// use null as a special value.
if (ret === undefined) {
if (hook.hook_fn.length >= 3) {
// The hook function has a callback parameter and it returned undefined, which means the
// hook function will settle (or has already settled) via the provided callback.
return;
} else {
// The hook function is assumed to not have a callback parameter, so fall through and accept
// `undefined` as the resolved value.
//
// IMPORTANT: "Rest" parameters and default parameters are not counted in `Function.length`,
// so the assumption does not hold for wrappers like `(...args) => { real(...args); }`. For
// such functions, calling the callback will (eventually) cause a double settle error, and
// the function might prematurely resolve to `undefined` instead of the desired value.
}
}
// Wrap ret in a Promise so that hook functions can be async (or otherwise return a Promise).
// Note: If ret is a Promise (or other thenable), Promise.resolve() will flatten it into this
// new Promise.
Promise.resolve(ret).then(
(val) => settle(null, val, 'returned value'),
(err) => settle(err, null, 'Promise rejection'));
});
}
// Invokes all registered hook functions asynchronously.
//
// Arguments:
// * hookName: Name of the hook to invoke.
// * context: Passed unmodified to the hook functions, except nullish becomes {}.
// * cb: Deprecated callback. The following:
// const p1 = hooks.aCallAll('myHook', context, cb);
// is equivalent to:
// const p2 = hooks.aCallAll('myHook', context).then((val) => cb(null, val), cb);
//
// Return value:
// If cb is nullish, this function resolves to a flattened array of hook results. Specifically, it
// is equivalent to doing the following:
// 1. Collect all values returned by the hook functions into an array.
// 2. Convert each `undefined` entry into `[]`.
// 3. Flatten one level.
// If cb is non-null, this function resolves to the value returned by cb.
exports.aCallAll = async (hookName, context, cb) => {
if (context == null) context = {};
const hooks = pluginDefs.hooks[hookName] || [];
let resultsPromise = Promise.all(hooks.map((hook) => {
return callHookFnAsync(hook, context)
// `undefined` (but not `null`!) is treated the same as [].
.then((result) => (result === undefined) ? [] : result);
})).then((results) => _.flatten(results, 1));
if (cb != null) resultsPromise = resultsPromise.then((val) => cb(null, val), cb);
return await resultsPromise;
};
exports.callFirst = function (hook_name, args) {
if (!args) args = {};
if (pluginDefs.hooks[hook_name] === undefined) return [];
@ -165,3 +405,9 @@ exports.callAllStr = function(hook_name, args, sep, pre, post) {
}
return newCallhooks.join(sep || "");
}
exports.exportedForTestingOnly = {
callHookFnAsync,
callHookFnSync,
deprecationWarned,
};