Merge pull request #2936 from node-red/npm-install-hooks

Add pre/postInstall hooks to npm install handling

packages/node_modules/@node-red/registry/lib/externalModules.js

@@ -9,6 +9,7 @@ const path = require("path");
 const clone = require("clone");
 const exec = require("@node-red/util").exec;
 const log = require("@node-red/util").log;
+const hooks = require("@node-red/util").hooks;
 
 const BUILTIN_MODULES = require('module').builtinModules;
 const EXTERNAL_MODULES_DIR = "externalModules";
@@ -189,13 +190,29 @@ async function installModule(moduleDetails) {
 
     await ensureModuleDir();
 
-    var args = ["install", installSpec, "--production"];
+    let triggerPayload = {
-    return exec.run(NPM_COMMAND, args, {
+        "module": moduleDetails.module,
-        cwd: installDir
+        "version": moduleDetails.version,
-    },true).then(result => {
+        "dir": installDir,
+        "args": ["--production"]
+    }
+    return hooks.trigger("preInstall", triggerPayload).then((result) => {
+        // preInstall passed
+        // - run install
+        if (result !== false) {
+            let extraArgs = triggerPayload.args || [];
+            let args = ['install', ...extraArgs, installSpec]
+            log.trace(NPM_COMMAND + JSON.stringify(args));
+            return exec.run(NPM_COMMAND, args, { cwd: installDir },true)
+        } else {
+            log.trace("skipping npm install");
+        }
+    }).then(() => {
+        return hooks.trigger("postInstall", triggerPayload)
+    }).then(() => {
         log.info(log._("server.install.installed", { name: installSpec }));
     }).catch(result => {
-        var output = result.stderr;
+        var output = result.stderr || result.toString();
         var e;
         if (/E404/.test(output) || /ETARGET/.test(output)) {
             log.error(log._("server.install.install-failed-not-found",{name:installSpec}));

packages/node_modules/@node-red/registry/lib/installer.js

@@ -23,7 +23,7 @@ const tar = require("tar");
 const registry = require("./registry");
 const registryUtil = require("./util");
 const library = require("./library");
-const {exec,log,events} = require("@node-red/util");
+const {exec,log,events,hooks} = require("@node-red/util");
 const child_process = require('child_process');
 const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm';
 let installerEnabled = false;
@@ -168,11 +168,30 @@ async function installModule(module,version,url) {
         }
 
         var installDir = settings.userDir || process.env.NODE_RED_HOME || ".";
-        var args = ['install','--no-audit','--no-update-notifier','--no-fund','--save','--save-prefix=~','--production',installName];
+        let triggerPayload = {
-        log.trace(npmCommand + JSON.stringify(args));
+            "module": module,
-        return exec.run(npmCommand,args,{
+            "version": version,
-            cwd: installDir
+            "url": url,
-        }, true).then(result => {
+            "dir": installDir,
+            "isExisting": isExisting,
+            "isUpgrade": isUpgrade,
+            "args": ['--no-audit','--no-update-notifier','--no-fund','--save','--save-prefix=~','--production']
+        }
+
+        return hooks.trigger("preInstall", triggerPayload).then((result) => {
+            // preInstall passed
+            // - run install
+            if (result !== false) {
+                let extraArgs = triggerPayload.args || [];
+                let args = ['install', ...extraArgs, installName]
+                log.trace(npmCommand + JSON.stringify(args));
+                return exec.run(npmCommand,args,{ cwd: installDir}, true)
+            } else {
+                log.trace("skipping npm install");
+            }
+        }).then(() => {
+            return hooks.trigger("postInstall", triggerPayload)
+        }).then(() => {
             if (isExisting) {
                 // This is a module we already have installed as a non-user module.
                 // That means it was discovered when loading, but was not listed
@@ -191,29 +210,45 @@ async function installModule(module,version,url) {
                 events.emit("runtime-event",{id:"restart-required",payload:{type:"warning",text:"notification.warnings.restartRequired"},retain:true});
                 return require("./registry").setModulePendingUpdated(module,version);
             }
-        }).catch(result => {
+        }).catch(err => {
-            var output = result.stderr;
+            let e;
-            var e;
+            if (err.hook) {
-            var lookFor404 = new RegExp(" 404 .*"+module,"m");
+                // preInstall failed
-            var lookForVersionNotFound = new RegExp("version not found: "+module+"@"+version,"m");
-            if (lookFor404.test(output)) {
-                log.warn(log._("server.install.install-failed-not-found",{name:module}));
-                e = new Error("Module not found");
-                e.code = 404;
-                throw e;
-            } else if (isUpgrade && lookForVersionNotFound.test(output)) {
-                log.warn(log._("server.install.upgrade-failed-not-found",{name:module}));
-                e = new Error("Module not found");
-                e.code = 404;
-                throw e;
-            } else {
                 log.warn(log._("server.install.install-failed-long",{name:module}));
                 log.warn("------------------------------------------");
-                log.warn(output);
+                log.warn(err.toString());
                 log.warn("------------------------------------------");
-                throw new Error(log._("server.install.install-failed"));
+                e = new Error(log._("server.install.install-failed")+": "+err.toString());
+                if (err.hook === "postInstall") {
+                    return exec.run(npmCommand,["remove",module],{ cwd: installDir}, false).finally(() => {
+                        throw e;
+                    })
+                }
+            } else {
+                // npm install failed
+                let output = err.stderr;
+                let lookFor404 = new RegExp(" 404 .*"+module,"m");
+                let lookForVersionNotFound = new RegExp("version not found: "+module+"@"+version,"m");
+                if (lookFor404.test(output)) {
+                    log.warn(log._("server.install.install-failed-not-found",{name:module}));
+                    e = new Error("Module not found");
+                    e.code = 404;
+                } else if (isUpgrade && lookForVersionNotFound.test(output)) {
+                    log.warn(log._("server.install.upgrade-failed-not-found",{name:module}));
+                    e = new Error("Module not found");
+                    e.code = 404;
+                } else {
+                    log.warn(log._("server.install.install-failed-long",{name:module}));
+                    log.warn("------------------------------------------");
+                    log.warn(output);
+                    log.warn("------------------------------------------");
+                    e = new Error(log._("server.install.install-failed"));
+                }
             }
-        })
+            if (e) {
+                throw e;
+            }
+        });
     }).catch(err => {
         // In case of error, reset activePromise to be resolvable
         activePromise = Promise.resolve();
@@ -412,17 +447,29 @@ function uninstallModule(module) {
             log.info(log._("server.install.uninstalling",{name:module}));
 
             var args = ['remove','--no-audit','--no-update-notifier','--no-fund','--save',module];
-            log.trace(npmCommand + JSON.stringify(args));
 
-            exec.run(npmCommand,args,{
+            let triggerPayload = {
-                cwd: installDir,
+                "module": module,
-            },true).then(result => {
+                "dir": installDir,
+            }
+            return hooks.trigger("preUninstall", triggerPayload).then(() => {
+                // preUninstall passed
+                // - run uninstall
+                log.trace(npmCommand + JSON.stringify(args));
+                return exec.run(npmCommand,args,{ cwd: installDir}, true)
+            }).then(() => {
                 log.info(log._("server.install.uninstalled",{name:module}));
                 reportRemovedModules(list);
                 library.removeExamplesDir(module);
-                resolve(list);
+                return hooks.trigger("postUninstall", triggerPayload).catch((err)=>{
+                    log.warn("------------------------------------------");
+                    log.warn(err.toString());
+                    log.warn("------------------------------------------");
+                }).finally(() => {
+                    resolve(list);
+                })
             }).catch(result => {
-                var output = result.stderr;
+                let output = result.stderr || result;
                 log.warn(log._("server.install.uninstall-failed-long",{name:module}));
                 log.warn("------------------------------------------");
                 log.warn(output.toString());

packages/node_modules/@node-red/runtime/lib/flows/Flow.js

@@ -19,7 +19,7 @@ var redUtil = require("@node-red/util").util;
 const events = require("@node-red/util").events;
 var flowUtil = require("./util");
 const context = require('../nodes/context');
-const hooks = require("../hooks");
+const hooks = require("@node-red/util").hooks;
 
 var Subflow;
 var Log;

packages/node_modules/@node-red/runtime/lib/index.js

@@ -20,7 +20,6 @@ var redNodes = require("./nodes");
 var flows = require("./flows");
 var storage = require("./storage");
 var library = require("./library");
-var hooks = require("./hooks");
 var plugins = require("./plugins");
 var settings = require("./settings");
 
@@ -29,7 +28,7 @@ var path = require('path');
 var fs = require("fs");
 var os = require("os");
 
-const {log,i18n,events,exec,util} = require("@node-red/util");
+const {log,i18n,events,exec,util,hooks} = require("@node-red/util");
 
 var runtimeMetricInterval = null;
 

packages/node_modules/@node-red/runtime/lib/nodes/Node.js

@@ -21,7 +21,7 @@ var redUtil = require("@node-red/util").util;
 var Log = require("@node-red/util").log;
 var context = require("./context");
 var flows = require("../flows");
-const hooks = require("../hooks");
+const hooks = require("@node-red/util").hooks;
 
 
 const NOOP_SEND = function() {}

packages/node_modules/@node-red/runtime/locales/en-US/runtime.json

@@ -34,6 +34,7 @@
             "install-failed-not-found": "$t(server.install.install-failed-long) module not found",
             "install-failed-name": "$t(server.install.install-failed-long) invalid module name: __name__",
             "install-failed-url": "$t(server.install.install-failed-long) invalid url: __url__",
+            "post-install-error": "Error running 'postInstall' hook:",
             "upgrading": "Upgrading module: __name__ to version: __version__",
             "upgraded": "Upgraded module: __name__. Restart Node-RED to use the new version",
             "upgrade-failed-not-found": "$t(server.install.install-failed-long) version not found",

packages/node_modules/@node-red/util/index.js

@@ -19,6 +19,7 @@ const i18n = require("./lib/i18n");
 const util = require("./lib/util");
 const events = require("./lib/events");
 const exec = require("./lib/exec");
+const hooks = require("./lib/hooks");
 
 /**
  * This module provides common utilities for the Node-RED runtime and editor
@@ -69,5 +70,12 @@ module.exports = {
     * @mixes @node-red/util_exec
     * @memberof @node-red/util
     */
-    exec: exec
+    exec: exec,
+
+    /**
+    * Runtime hooks
+    * @mixes @node-red/util_hooks
+    * @memberof @node-red/util
+    */
+    hooks: hooks
 }

packages/node_modules/@node-red/util/lib/hooks.js

@@ -1,4 +1,4 @@
-const Log = require("@node-red/util").log;
+const Log = require("./log.js");
 
 const VALID_HOOKS = [
     // Message Routing Path
@@ -8,14 +8,28 @@ const VALID_HOOKS = [
    "postDeliver",
    "onReceive",
    "postReceive",
-   "onComplete"
+   "onComplete",
+   // Module install hooks
+   "preInstall",
+   "postInstall",
+   "preUninstall",
+   "postUninstall"
 ]
 
 
 // Flags for what hooks have handlers registered
 let states = { }
 
-// Hooks by id
+// Doubly-LinkedList of hooks by id.
+// - hooks[id] points to head of list
+// - each list item looks like:
+//   {
+//       cb: the callback function
+//       location: filename/line of code that added the hook
+//       previousHook: reference to previous hook in list
+//       nextHook: reference to next hook in list
+//       removed: a flag that is set if the item was removed
+//   }
 let hooks = { }
 
 // Hooks by label
@@ -35,12 +49,12 @@ let labelledHooks = { }
  *  - `postReceive` - passed a `ReceiveEvent` when the message has been given to the node's `input` handler(s)
  *  - `onComplete` - passed a `CompleteEvent` when the node has completed with a message or logged an error
  *
- * @mixin @node-red/runtime_hooks
+ * @mixin @node-red/util_hooks
  */
 
 /**
  * Register a handler to a named hook
- * @memberof @node-red/runtime_hooks
+ * @memberof @node-red/util_hooks
  * @param {String} hookId - the name of the hook to attach to
  * @param {Function} callback - the callback function for the hook
  */
@@ -49,26 +63,39 @@ function add(hookId, callback) {
     if (VALID_HOOKS.indexOf(id) === -1) {
         throw new Error(`Invalid hook '${id}'`);
     }
-    if (label) {
+    if (label && labelledHooks[label] && labelledHooks[label][id]) {
-        if (labelledHooks[label] && labelledHooks[label][id]) {
+        throw new Error("Hook "+hookId+" already registered")
-            throw new Error("Hook "+hookId+" already registered")
-        }
-        labelledHooks[label] = labelledHooks[label]||{};
-        labelledHooks[label][id] = callback;
     }
     // Get location of calling code
     const stack = new Error().stack;
     const callModule = stack.split("\n")[2].split("(")[1].slice(0,-1);
     Log.debug(`Adding hook '${hookId}' from ${callModule}`);
 
-    hooks[id] = hooks[id] || [];
+    const hookItem = {cb:callback, location: callModule, previousHook: null, nextHook: null }
-    hooks[id].push({cb:callback,location:callModule});
+
+    let tailItem = hooks[id];
+    if (tailItem === undefined) {
+        hooks[id] = hookItem;
+    } else {
+        while(tailItem.nextHook !== null) {
+            tailItem = tailItem.nextHook
+        }
+        tailItem.nextHook = hookItem;
+        hookItem.previousHook = tailItem;
+    }
+
+    if (label) {
+        labelledHooks[label] = labelledHooks[label]||{};
+        labelledHooks[label][id] = hookItem;
+    }
+
+    // TODO: get rid of this;
     states[id] = true;
 }
 
 /**
  * Remove a handled from a named hook
- * @memberof @node-red/runtime_hooks
+ * @memberof @node-red/util_hooks
  * @param {String} hookId - the name of the hook event to remove - must be `name.label`
  */
 function remove(hookId) {
@@ -95,33 +122,66 @@ function remove(hookId) {
     }
 }
 
-function removeHook(id,callback) {
+function removeHook(id,hookItem) {
-    let i = hooks[id].findIndex(hook => hook.cb === callback);
+    let previousHook = hookItem.previousHook;
-    if (i !== -1) {
+    let nextHook = hookItem.nextHook;
-        hooks[id].splice(i,1);
+
-        if (hooks[id].length === 0) {
+    if (previousHook) {
-            delete hooks[id];
+        previousHook.nextHook = nextHook;
-            delete states[id];
+    } else {
-        }
+        hooks[id] = nextHook;
+    }
+    if (nextHook) {
+        nextHook.previousHook = previousHook;
+    }
+    hookItem.removed = true;
+    if (!previousHook && !nextHook) {
+        delete hooks[id];
+        delete states[id];
     }
 }
 
 
 function trigger(hookId, payload, done) {
-    const hookStack = hooks[hookId];
+    let hookItem = hooks[hookId];
-    if (!hookStack || hookStack.length === 0) {
+    if (!hookItem) {
-        done();
+        if (done) {
-        return;
+            done();
+            return;
+        } else {
+            return Promise.resolve();
+        }
     }
-    let i = 0;
+    if (!done) {
-
+        return new Promise((resolve,reject) => {
+            invokeStack(hookItem,payload,function(err) {
+                if (err !== undefined && err !== false) {
+                    if (!(err instanceof Error)) {
+                        err = new Error(err);
+                    }
+                    err.hook = hookId
+                    reject(err);
+                } else {
+                    resolve(err);
+                }
+            })
+        });
+    } else {
+        invokeStack(hookItem,payload,done)
+    }
+}
+function invokeStack(hookItem,payload,done) {
     function callNextHook(err) {
-        if (i === hookStack.length || err) {
+        if (!hookItem || err) {
             done(err);
             return;
         }
-        const hook = hookStack[i++];
+        if (hookItem.removed) {
-        const callback = hook.cb;
+            hookItem = hookItem.nextHook;
+            callNextHook();
+            return;
+        }
+        const callback = hookItem.cb;
         if (callback.length === 1) {
             try {
                 let result = callback(payload);
@@ -134,6 +194,7 @@ function trigger(hookId, payload, done) {
                     result.then(handleResolve, callNextHook)
                     return;
                 }
+                hookItem = hookItem.nextHook;
                 callNextHook();
             } catch(err) {
                 done(err);
@@ -148,15 +209,15 @@ function trigger(hookId, payload, done) {
             }
         }
     }
-    callNextHook();
-
     function handleResolve(result) {
         if (result === undefined) {
+            hookItem = hookItem.nextHook;
             callNextHook();
         } else {
             done(result);
         }
     }
+    callNextHook();
 }
 
 function clear() {
@@ -179,4 +240,4 @@ module.exports = {
     add,
     remove,
     trigger
-}
+}

test/unit/@node-red/registry/lib/externalModules_spec.js

@@ -14,6 +14,7 @@ const os = require("os");
 const NR_TEST_UTILS = require("nr-test-utils");
 const externalModules = NR_TEST_UTILS.require("@node-red/registry/lib/externalModules");
 const exec = NR_TEST_UTILS.require("@node-red/util/lib/exec");
+const hooks = NR_TEST_UTILS.require("@node-red/util/lib/hooks");
 
 let homeDir;
 
@@ -40,19 +41,20 @@ describe("externalModules api", function() {
         await createUserDir()
     })
     afterEach(async function() {
+        hooks.clear();
         await fs.remove(homeDir);
     })
     describe("checkFlowDependencies", function() {
         beforeEach(function() {
             sinon.stub(exec,"run").callsFake(async function(cmd, args, options) {
                 let error;
-                if (args[1] === "moduleNotFound") {
+                if (args[2] === "moduleNotFound") {
                     error = new Error();
                     error.stderr = "E404";
-                } else if (args[1] === "moduleVersionNotFound") {
+                } else if (args[2] === "moduleVersionNotFound") {
                     error = new Error();
                     error.stderr = "ETARGET";
-                } else if (args[1] === "moduleFail") {
+                } else if (args[2] === "moduleFail") {
                     error = new Error();
                     error.stderr = "Some unexpected install error";
                 }
@@ -102,6 +104,45 @@ describe("externalModules api", function() {
             fs.existsSync(path.join(homeDir,"externalModules")).should.be.true();
         })
 
+
+        it("calls pre/postInstall hooks", async function() {
+            externalModules.init({userDir: homeDir});
+            externalModules.register("function", "libs");
+            let receivedPreEvent,receivedPostEvent;
+            hooks.add("preInstall", function(event) { event.args = ["a"]; receivedPreEvent = event; })
+            hooks.add("postInstall", function(event) { receivedPostEvent = event; })
+
+            await externalModules.checkFlowDependencies([
+                {type: "function", libs:[{module: "foo"}]}
+            ])
+            exec.run.called.should.be.true();
+            // exec.run.lastCall.args[1].should.eql([ 'install', 'a', 'foo' ]);
+            receivedPreEvent.should.have.property("module","foo")
+            receivedPreEvent.should.have.property("version")
+            receivedPreEvent.should.have.property("dir")
+            receivedPreEvent.should.eql(receivedPostEvent)
+            fs.existsSync(path.join(homeDir,"externalModules")).should.be.true();
+        })
+
+        it("skips npm install if preInstall returns false", async function() {
+            externalModules.init({userDir: homeDir});
+            externalModules.register("function", "libs");
+            let receivedPreEvent,receivedPostEvent;
+            hooks.add("preInstall", function(event) { receivedPreEvent = event; return false })
+            hooks.add("postInstall", function(event) { receivedPostEvent = event; })
+
+            await externalModules.checkFlowDependencies([
+                {type: "function", libs:[{module: "foo"}]}
+            ])
+            exec.run.called.should.be.false();
+            receivedPreEvent.should.have.property("module","foo")
+            receivedPreEvent.should.have.property("version")
+            receivedPreEvent.should.have.property("dir")
+            receivedPreEvent.should.eql(receivedPostEvent)
+            fs.existsSync(path.join(homeDir,"externalModules")).should.be.true();
+        })
+
+
         it("installs missing modules from inside subflow module", async function() {
             externalModules.init({userDir: homeDir});
             externalModules.register("function", "libs");
@@ -299,4 +340,4 @@ describe("externalModules api", function() {
             }
         })
     })
-});
+});

test/unit/@node-red/registry/lib/installer_spec.js

@@ -25,7 +25,7 @@ var NR_TEST_UTILS = require("nr-test-utils");
 var installer = NR_TEST_UTILS.require("@node-red/registry/lib/installer");
 var registry = NR_TEST_UTILS.require("@node-red/registry/lib/index");
 var typeRegistry = NR_TEST_UTILS.require("@node-red/registry/lib/registry");
-const { events, exec, log } =  NR_TEST_UTILS.require("@node-red/util");
+const { events, exec, log, hooks } =  NR_TEST_UTILS.require("@node-red/util");
 
 describe('nodes/registry/installer', function() {
 
@@ -68,6 +68,7 @@ describe('nodes/registry/installer', function() {
             fs.statSync.restore();
         }
         exec.run.restore();
+        hooks.clear();
     });
 
     describe("installs module", function() {
@@ -251,6 +252,70 @@ describe('nodes/registry/installer', function() {
             }).catch(done);
         });
 
+        it("triggers preInstall and postInstall hooks", function(done) {
+            let receivedPreEvent,receivedPostEvent;
+            hooks.add("preInstall", function(event) { event.args = ["a"]; receivedPreEvent = event; })
+            hooks.add("postInstall", function(event) { receivedPostEvent = event; })
+            var nodeInfo = {nodes:{module:"foo",types:["a"]}};
+            var res = {code: 0,stdout:"",stderr:""}
+            var p = Promise.resolve(res);
+            p.catch((err)=>{});
+            execResponse = p;
+
+            var addModule = sinon.stub(registry,"addModule").callsFake(function(md) {
+                return Promise.resolve(nodeInfo);
+            });
+
+            installer.installModule("this_wont_exist","1.2.3").then(function(info) {
+                exec.run.called.should.be.true();
+                exec.run.lastCall.args[1].should.eql([ 'install', 'a', 'this_wont_exist@1.2.3' ]);
+                info.should.eql(nodeInfo);
+                should.exist(receivedPreEvent)
+                receivedPreEvent.should.have.property("module","this_wont_exist")
+                receivedPreEvent.should.have.property("version","1.2.3")
+                receivedPreEvent.should.have.property("dir")
+                receivedPreEvent.should.have.property("url")
+                receivedPreEvent.should.have.property("isExisting")
+                receivedPreEvent.should.have.property("isUpgrade")
+                receivedPreEvent.should.eql(receivedPostEvent)
+                done();
+            }).catch(done);
+        });
+
+        it("fails install if preInstall hook fails", function(done) {
+            let receivedEvent;
+            hooks.add("preInstall", function(event) { throw new Error("preInstall-error"); })
+            var nodeInfo = {nodes:{module:"foo",types:["a"]}};
+
+            installer.installModule("this_wont_exist","1.2.3").catch(function(err) {
+                exec.run.called.should.be.false();
+                done();
+            }).catch(done);
+        });
+
+        it("skips invoking npm if preInstall returns false", function(done) {
+            let receivedEvent;
+            hooks.add("preInstall", function(event) { return false })
+            hooks.add("postInstall", function(event) { receivedEvent = event; })
+            var nodeInfo = {nodes:{module:"foo",types:["a"]}};
+
+            installer.installModule("this_wont_exist","1.2.3").catch(function(err) {
+                exec.run.called.should.be.false();
+                should.exist(receivedEvent);
+                done();
+            }).catch(done);
+        });
+
+        it("rollsback install if postInstall hook fails", function(done) {
+            hooks.add("postInstall", function(event) { throw new Error("fail"); })
+            installer.installModule("this_wont_exist","1.2.3").catch(function(err) {
+                exec.run.calledTwice.should.be.true();
+                exec.run.firstCall.args[1].includes("install").should.be.true();
+                exec.run.secondCall.args[1].includes("remove").should.be.true();
+                done();
+            }).catch(done);
+        });
+
     });
     describe("uninstalls module", function() {
         it("rejects invalid module names", function(done) {

test/unit/@node-red/runtime/lib/flows/Flow_spec.js

@@ -26,7 +26,7 @@ var flowUtils = NR_TEST_UTILS.require("@node-red/runtime/lib/flows/util");
 var Flow = NR_TEST_UTILS.require("@node-red/runtime/lib/flows/Flow");
 var flows = NR_TEST_UTILS.require("@node-red/runtime/lib/flows");
 var Node = NR_TEST_UTILS.require("@node-red/runtime/lib/nodes/Node");
-var hooks = NR_TEST_UTILS.require("@node-red/runtime/lib/hooks");
+var hooks = NR_TEST_UTILS.require("@node-red/util/lib/hooks");
 var typeRegistry = NR_TEST_UTILS.require("@node-red/registry");
 
 

test/unit/@node-red/runtime/lib/nodes/Node_spec.js

@@ -19,7 +19,7 @@ var sinon = require('sinon');
 var NR_TEST_UTILS = require("nr-test-utils");
 var RedNode = NR_TEST_UTILS.require("@node-red/runtime/lib/nodes/Node");
 var Log = NR_TEST_UTILS.require("@node-red/util").log;
-var hooks = NR_TEST_UTILS.require("@node-red/runtime/lib/hooks");
+var hooks = NR_TEST_UTILS.require("@node-red/util/lib/hooks");
 var flows = NR_TEST_UTILS.require("@node-red/runtime/lib/flows");
 
 describe('Node', function() {

test/unit/@node-red/util/lib/hooks_spec.js

@@ -1,9 +1,9 @@
 const should = require("should");
 const NR_TEST_UTILS = require("nr-test-utils");
 
-const hooks = NR_TEST_UTILS.require("@node-red/runtime/lib/hooks");
+const hooks = NR_TEST_UTILS.require("@node-red/util/lib/hooks");
 
-describe("runtime/hooks", function() {
+describe("util/hooks", function() {
     afterEach(function() {
         hooks.clear();
     })
@@ -81,7 +81,7 @@ describe("runtime/hooks", function() {
                 hooks.has("onSend.A").should.be.false();
                 hooks.has("onSend.B").should.be.false();
                 hooks.has("onSend").should.be.false();
-                
+
                 done(err);
             } catch(err2) {
                 done(err2);
@@ -121,7 +121,46 @@ describe("runtime/hooks", function() {
             })
         })
     })
+    it("allows a hook to remove itself whilst being called", function(done) {
+        let data = { order: [] }
+        hooks.add("onSend.A", function(payload) { payload.order.push("A") } )
+        hooks.add("onSend.B", function(payload) {
+            hooks.remove("*.B");
+        })
+        hooks.add("onSend.C", function(payload) { payload.order.push("C") } )
+        hooks.add("onSend.D", function(payload) { payload.order.push("D") } )
 
+        hooks.trigger("onSend", data, err => {
+            try {
+                should.not.exist(err);
+                data.order.should.eql(["A","C","D"])
+                done();
+            } catch(e) {
+                done(e);
+            }
+        })
+    });
+
+    it("allows a hook to remove itself and others whilst being called", function(done) {
+        let data = { order: [] }
+        hooks.add("onSend.A", function(payload) { payload.order.push("A") } )
+        hooks.add("onSend.B", function(payload) {
+            hooks.remove("*.B");
+            hooks.remove("*.C");
+        })
+        hooks.add("onSend.C", function(payload) { payload.order.push("C") } )
+        hooks.add("onSend.D", function(payload) { payload.order.push("D") } )
+
+        hooks.trigger("onSend", data, err => {
+            try {
+                should.not.exist(err);
+                data.order.should.eql(["A","D"])
+                done();
+            } catch(e) {
+                done(e);
+            }
+        })
+    });
 
     it("halts execution on return false", function(done) {
         hooks.add("onSend.A", function(payload) { payload.order.push("A"); return false } )
@@ -249,4 +288,51 @@ describe("runtime/hooks", function() {
             done();
         })
     })
+
+
+    it("handler can use callback function - promise API", function(done) {
+        hooks.add("onSend.A", function(payload, done) {
+            setTimeout(function() {
+                payload.order.push("A")
+                done()
+            },30)
+        })
+        hooks.add("onSend.B", function(payload) { payload.order.push("B") } )
+
+        let data = { order:[] };
+        hooks.trigger("onSend",data).then(() => {
+            data.order.should.eql(["A","B"])
+            done()
+        }).catch(done)
+    })
+
+    it("handler can halt execution - promise API", function(done) {
+        hooks.add("onSend.A", function(payload, done) {
+            setTimeout(function() {
+                payload.order.push("A")
+                done(false)
+            },30)
+        })
+        hooks.add("onSend.B", function(payload) { payload.order.push("B") } )
+
+        let data = { order:[] };
+        hooks.trigger("onSend",data).then(() => {
+            data.order.should.eql(["A"])
+            done()
+        }).catch(done)
+    })
+
+    it("handler can halt execution on error - promise API", function(done) {
+        hooks.add("onSend.A", function(payload, done) {
+            throw new Error("error");
+        })
+        hooks.add("onSend.B", function(payload) { payload.order.push("B") } )
+
+        let data = { order:[] };
+        hooks.trigger("onSend",data).then(() => {
+            done("hooks.trigger resolved unexpectedly")
+        }).catch(err => {
+            done();
+        })
+    })
 });