Bug 1440777 - Add support for local actions and implement console-log as a local action r=Gijs

* Add ActionsManager to provide a common interface to local and remote actions
* Move action handle logic from RecipeRunner to new ActionsManager
* Implement BaseAction for all local actions
* Implement ConsoleLog as a subclass of BaseAction
* Validate action arguments with schema validator from PolicyEngine

MozReview-Commit-ID: E2cxkkvYjCz

--HG--
extra : rebase_source : 8e68674a08011208dad0f763fe1867df6808d837

toolkit/components/normandy/actions/BaseAction.jsm

@@ -0,0 +1,138 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+ChromeUtils.defineModuleGetter(this, "LogManager", "resource://normandy/lib/LogManager.jsm");
+ChromeUtils.defineModuleGetter(this, "Uptake", "resource://normandy/lib/Uptake.jsm");
+ChromeUtils.defineModuleGetter(this, "PoliciesValidator", "resource:///modules/policies/PoliciesValidator.jsm");
+
+var EXPORTED_SYMBOLS = ["BaseAction"];
+
+/**
+ * Base class for local actions.
+ *
+ * This should be subclassed. Subclasses must implement _run() for
+ * per-recipe behavior, and may implement _preExecution and _finalize
+ * for actions to be taken once before and after recipes are run.
+ *
+ * Other methods should be overridden with care, to maintain the life
+ * cycle events and error reporting implemented by this class.
+ */
+class BaseAction {
+  constructor() {
+    this.finalized = false;
+    this.failed = false;
+    this.log = LogManager.getLogger(`action.${this.name}`);
+
+    try {
+      this._preExecution();
+    } catch (err) {
+      this.failed = true;
+      this.log.error(`Could not initialize action ${this.name}: ${err}`);
+      Uptake.reportAction(this.name, Uptake.ACTION_PRE_EXECUTION_ERROR);
+    }
+  }
+
+  get schema() {
+    return {
+      type: "object",
+      properties: {},
+    };
+  }
+
+  // Gets the name of the action. Does not necessarily match the
+  // server slug for the action.
+  get name() {
+    return this.constructor.name;
+  }
+
+  /**
+   * Action specific pre-execution behavior should be implemented
+   * here. It will be called once per execution session.
+   */
+  _preExecution() {
+    // Does nothing, may be overridden
+  }
+
+  /**
+   * Execute the per-recipe behavior of this action for a given
+   * recipe.  Reports Uptake telemetry for the execution of the recipe.
+   *
+   * @param {Recipe} recipe
+   * @throws If this action has already been finalized.
+   */
+  async runRecipe(recipe) {
+    if (this.finalized) {
+      throw new Error("Action has already been finalized");
+    }
+
+    if (this.failed) {
+      Uptake.reportRecipe(recipe.id, Uptake.RECIPE_ACTION_DISABLED);
+      this.log.warn(`Skipping recipe ${recipe.name} because ${this.name} failed during preExecution.`);
+      return;
+    }
+
+    let [valid, validatedArguments] = PoliciesValidator.validateAndParseParameters(recipe.arguments, this.schema);
+    if (!valid) {
+      Cu.reportError(new Error(`Arguments do not match schema. arguments: ${JSON.stringify(recipe.arguments)}. schema: ${JSON.stringify(this.schema)}`));
+      Uptake.reportRecipe(recipe.id, Uptake.RECIPE_EXECUTION_ERROR);
+      return;
+    }
+
+    recipe.arguments = validatedArguments;
+
+    let status = Uptake.RECIPE_SUCCESS;
+    try {
+      await this._run(recipe);
+    } catch (err) {
+      this.log.error(`Could not execute recipe ${recipe.name}: ${err}`);
+      status = Uptake.RECIPE_EXECUTION_ERROR;
+    }
+    Uptake.reportRecipe(recipe.id, status);
+  }
+
+  /**
+   * Action specific recipe behavior must be implemented here. It
+   * will be executed once for reach recipe, being passed the recipe
+   * as a parameter.
+   */
+  async _run(recipe) {
+    throw new Error("Not implemented");
+  }
+
+  /**
+   * Finish an execution session. After this method is called, no
+   * other methods may be called on this method, and all relevant
+   * recipes will be assumed to have been seen.
+   */
+  async finalize() {
+    if (this.finalized) {
+      throw new Error("Action has already been finalized");
+    }
+
+    if (this.failed) {
+      this.log.info(`Skipping post-execution hook for ${this.name} due to earlier failure.`);
+      return;
+    }
+
+    let status = Uptake.ACTION_SUCCESS;
+    try {
+      this._finalize();
+    } catch (err) {
+      status = Uptake.ACTION_POST_EXECUTION_ERROR;
+      this.log.info(`Could not run postExecution hook for ${this.name}: ${err.message}`);
+    } finally {
+      this.finalized = true;
+      Uptake.reportAction(this.name, status);
+    }
+  }
+
+  /**
+   * Action specific post-execution behavior should be implemented
+   * here. It will be executed once after all recipes have been
+   * processed.
+   */
+  _finalize() {
+    // Does nothing, may be overridden
+  }
+}

toolkit/components/normandy/actions/ConsoleLog.jsm

@@ -0,0 +1,20 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+ChromeUtils.import("resource://normandy/actions/BaseAction.jsm");
+ChromeUtils.defineModuleGetter(this, "ActionSchemas", "resource://normandy/actions/schemas/index.js");
+
+var EXPORTED_SYMBOLS = ["ConsoleLog"];
+
+class ConsoleLog extends BaseAction {
+  get schema() {
+    return ActionSchemas.consoleLog;
+  }
+
+  async _run(recipe) {
+    this.log.info(recipe.arguments.message);
+  }
+}

toolkit/components/normandy/actions/schemas/README.md

@@ -0,0 +1,5 @@
+# Normandy Action Argument Schemas
+
+This is a collection of schemas describing the arguments expected by  Normandy
+actions. It's primary purpose is to be used in the Normandy server and Delivery
+Console to validate data and provide better user interactions.

toolkit/components/normandy/actions/schemas/index.js

@@ -0,0 +1,23 @@
+var EXPORTED_SYMBOLS = ["ActionSchemas"];
+
+const ActionSchemas = {
+  consoleLog: {
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "title": "Log a message to the console",
+    "type": "object",
+    "required": [
+      "message"
+    ],
+    "properties": {
+      "message": {
+        "description": "Message to log to the console",
+        "type": "string",
+        "default": ""
+      }
+    }
+  }
+};
+
+if (this.exports) {
+  this.exports = ActionSchemas;
+}

toolkit/components/normandy/actions/schemas/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "mozilla-normandy-action-argument-schemas",
+  "version": "0.1.0",
+  "description": "Schemas for Normandy action arguments",
+  "main": "index.js",
+  "author": "Michael Cooper <mcooper@mozilla.com>",
+  "license": "MPL-2.0"
+}

toolkit/components/normandy/jar.mn

@@ -7,6 +7,9 @@ toolkit.jar:
   res/normandy/Normandy.jsm (./Normandy.jsm)
   res/normandy/lib/ (./lib/*)
   res/normandy/skin/  (./skin/*)
+  res/normandy/actions/BaseAction.jsm (./actions/BaseAction.jsm)
+  res/normandy/actions/ConsoleLog.jsm (./actions/ConsoleLog.jsm)
+  res/normandy/actions/schemas/index.js (./actions/schemas/index.js)
 
 % resource normandy-content %res/normandy/content/ contentaccessible=yes
   res/normandy/content/ (./content/*)

toolkit/components/normandy/lib/ActionsManager.jsm

@@ -0,0 +1,148 @@
+ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+ChromeUtils.import("resource://normandy/lib/LogManager.jsm");
+
+XPCOMUtils.defineLazyModuleGetters(this, {
+  ActionSandboxManager: "resource://normandy/lib/ActionSandboxManager.jsm",
+  ConsoleLog: "resource://normandy/actions/ConsoleLog.jsm",
+  NormandyApi: "resource://normandy/lib/NormandyApi.jsm",
+  Uptake: "resource://normandy/lib/Uptake.jsm",
+});
+
+var EXPORTED_SYMBOLS = ["ActionsManager"];
+
+const log = LogManager.getLogger("recipe-runner");
+
+/**
+ * A class to manage the actions that recipes can use in Normandy.
+ *
+ * This includes both remote and local actions. Remote actions
+ * implementations are fetched from the Normandy server; their
+ * lifecycles are managed by `normandy/lib/ActionSandboxManager.jsm`.
+ * Local actions have their implementations packaged in the Normandy
+ * client, and manage their lifecycles internally.
+ */
+class ActionsManager {
+  constructor() {
+    this.finalized = false;
+    this.remoteActionSandboxes = {};
+
+    this.localActions = {
+      "console-log": new ConsoleLog(),
+    };
+  }
+
+  async fetchRemoteActions() {
+    const actions = await NormandyApi.fetchActions();
+
+    for (const action of actions) {
+      // Skip actions with local implementations
+      if (action.name in this.localActions) {
+        continue;
+      }
+
+      try {
+        const implementation = await NormandyApi.fetchImplementation(action);
+        const sandbox = new ActionSandboxManager(implementation);
+        sandbox.addHold("ActionsManager");
+        this.remoteActionSandboxes[action.name] = sandbox;
+      } catch (err) {
+        log.warn(`Could not fetch implementation for ${action.name}: ${err}`);
+
+        let status;
+        if (/NetworkError/.test(err)) {
+          status = Uptake.ACTION_NETWORK_ERROR;
+        } else {
+          status = Uptake.ACTION_SERVER_ERROR;
+        }
+        Uptake.reportAction(action.name, status);
+      }
+    }
+  }
+
+  async preExecution() {
+    // Local actions run pre-execution hooks implicitly
+
+    for (const [actionName, manager] of Object.entries(this.remoteActionSandboxes)) {
+      try {
+        await manager.runAsyncCallback("preExecution");
+        manager.disabled = false;
+      } catch (err) {
+        log.error(`Could not run pre-execution hook for ${actionName}:`, err.message);
+        manager.disabled = true;
+        Uptake.reportAction(actionName, Uptake.ACTION_PRE_EXECUTION_ERROR);
+      }
+    }
+  }
+
+  async runRecipe(recipe) {
+    let actionName = recipe.action;
+
+    if (actionName in this.localActions) {
+      log.info(`Executing recipe "${recipe.name}" (action=${recipe.action})`);
+      const action = this.localActions[actionName];
+      await action.runRecipe(recipe);
+
+    } else if (actionName in this.remoteActionSandboxes) {
+      let status;
+      const manager = this.remoteActionSandboxes[recipe.action];
+
+      if (manager.disabled) {
+        log.warn(
+          `Skipping recipe ${recipe.name} because ${recipe.action} failed during pre-execution.`
+        );
+        status = Uptake.RECIPE_ACTION_DISABLED;
+      } else {
+        try {
+          log.info(`Executing recipe "${recipe.name}" (action=${recipe.action})`);
+          await manager.runAsyncCallback("action", recipe);
+          status = Uptake.RECIPE_SUCCESS;
+        } catch (e) {
+          e.message = `Could not execute recipe ${recipe.name}: ${e.message}`;
+          Cu.reportError(e);
+          status = Uptake.RECIPE_EXECUTION_ERROR;
+        }
+      }
+      Uptake.reportRecipe(recipe.id, status);
+
+    } else {
+      log.error(
+        `Could not execute recipe ${recipe.name}:`,
+        `Action ${recipe.action} is either missing or invalid.`
+      );
+      Uptake.reportRecipe(recipe.id, Uptake.RECIPE_INVALID_ACTION);
+    }
+  }
+
+  async finalize() {
+    if (this.finalized) {
+      throw new Error("ActionsManager has already been finalized");
+    }
+    this.finalized = true;
+
+    // Finalize local actions
+    for (const action of Object.values(this.localActions)) {
+      action.finalize();
+    }
+
+    // Run post-execution hooks for remote actions
+    for (const [actionName, manager] of Object.entries(this.remoteActionSandboxes)) {
+      // Skip if pre-execution failed.
+      if (manager.disabled) {
+        log.info(`Skipping post-execution hook for ${actionName} due to earlier failure.`);
+        continue;
+      }
+
+      try {
+        await manager.runAsyncCallback("postExecution");
+        Uptake.reportAction(actionName, Uptake.ACTION_SUCCESS);
+      } catch (err) {
+        log.info(`Could not run post-execution hook for ${actionName}:`, err.message);
+        Uptake.reportAction(actionName, Uptake.ACTION_POST_EXECUTION_ERROR);
+      }
+    }
+
+    // Nuke sandboxes
+    Object.values(this.remoteActionSandboxes)
+      .forEach(manager => manager.removeHold("ActionsManager"));
+  }
+}

toolkit/components/normandy/lib/RecipeRunner.jsm

@@ -11,27 +11,20 @@ ChromeUtils.import("resource://normandy/lib/LogManager.jsm");
 XPCOMUtils.defineLazyServiceGetter(this, "timerManager",
                                    "@mozilla.org/updates/timer-manager;1",
                                    "nsIUpdateTimerManager");
-ChromeUtils.defineModuleGetter(this, "Preferences", "resource://gre/modules/Preferences.jsm");
-ChromeUtils.defineModuleGetter(this, "Storage",
-                               "resource://normandy/lib/Storage.jsm");
-ChromeUtils.defineModuleGetter(this, "NormandyDriver",
-                               "resource://normandy/lib/NormandyDriver.jsm");
-ChromeUtils.defineModuleGetter(this, "FilterExpressions",
-                               "resource://normandy/lib/FilterExpressions.jsm");
-ChromeUtils.defineModuleGetter(this, "NormandyApi",
-                               "resource://normandy/lib/NormandyApi.jsm");
-ChromeUtils.defineModuleGetter(this, "SandboxManager",
-                               "resource://normandy/lib/SandboxManager.jsm");
-ChromeUtils.defineModuleGetter(this, "ClientEnvironment",
-                               "resource://normandy/lib/ClientEnvironment.jsm");
-ChromeUtils.defineModuleGetter(this, "CleanupManager",
-                               "resource://normandy/lib/CleanupManager.jsm");
-ChromeUtils.defineModuleGetter(this, "ActionSandboxManager",
-                               "resource://normandy/lib/ActionSandboxManager.jsm");
-ChromeUtils.defineModuleGetter(this, "AddonStudies",
-                               "resource://normandy/lib/AddonStudies.jsm");
-ChromeUtils.defineModuleGetter(this, "Uptake",
-                               "resource://normandy/lib/Uptake.jsm");
+
+XPCOMUtils.defineLazyModuleGetters(this, {
+  Preferences: "resource://gre/modules/Preferences.jsm",
+  Storage: "resource://normandy/lib/Storage.jsm",
+  NormandyDriver: "resource://normandy/lib/NormandyDriver.jsm",
+  FilterExpressions: "resource://normandy/lib/FilterExpressions.jsm",
+  NormandyApi: "resource://normandy/lib/NormandyApi.jsm",
+  SandboxManager: "resource://normandy/lib/SandboxManager.jsm",
+  ClientEnvironment: "resource://normandy/lib/ClientEnvironment.jsm",
+  CleanupManager: "resource://normandy/lib/CleanupManager.jsm",
+  AddonStudies: "resource://normandy/lib/AddonStudies.jsm",
+  Uptake: "resource://normandy/lib/Uptake.jsm",
+  ActionsManager: "resource://normandy/lib/ActionsManager.jsm",
+});
 
 Cu.importGlobalProperties(["fetch"]);
 
@@ -212,21 +205,9 @@ var RecipeRunner = {
       return;
     }
 
-    const actionSandboxManagers = await this.loadActionSandboxManagers();
-    Object.values(actionSandboxManagers).forEach(manager => manager.addHold("recipeRunner"));
-
-    // Run pre-execution hooks. If a hook fails, we don't run recipes with that
-    // action to avoid inconsistencies.
-    for (const [actionName, manager] of Object.entries(actionSandboxManagers)) {
-      try {
-        await manager.runAsyncCallback("preExecution");
-        manager.disabled = false;
-      } catch (err) {
-        log.error(`Could not run pre-execution hook for ${actionName}:`, err.message);
-        manager.disabled = true;
-        Uptake.reportAction(actionName, Uptake.ACTION_PRE_EXECUTION_ERROR);
-      }
-    }
+    const actions = new ActionsManager();
+    await actions.fetchRemoteActions();
+    await actions.preExecution();
 
     // Evaluate recipe filters
     const recipesToRun = [];
@@ -241,53 +222,11 @@ var RecipeRunner = {
       log.debug("No recipes to execute");
     } else {
       for (const recipe of recipesToRun) {
-        const manager = actionSandboxManagers[recipe.action];
-        let status;
-        if (!manager) {
-          log.error(
-            `Could not execute recipe ${recipe.name}:`,
-            `Action ${recipe.action} is either missing or invalid.`
-          );
-          status = Uptake.RECIPE_INVALID_ACTION;
-        } else if (manager.disabled) {
-          log.warn(
-            `Skipping recipe ${recipe.name} because ${recipe.action} failed during pre-execution.`
-          );
-          status = Uptake.RECIPE_ACTION_DISABLED;
-        } else {
-          try {
-            log.info(`Executing recipe "${recipe.name}" (action=${recipe.action})`);
-            await manager.runAsyncCallback("action", recipe);
-            status = Uptake.RECIPE_SUCCESS;
-          } catch (err) {
-            log.error(`Could not execute recipe ${recipe.name}: ${err}`);
-            status = Uptake.RECIPE_EXECUTION_ERROR;
-          }
-        }
-
-        Uptake.reportRecipe(recipe.id, status);
+        actions.runRecipe(recipe);
       }
     }
 
-    // Run post-execution hooks
-    for (const [actionName, manager] of Object.entries(actionSandboxManagers)) {
-      // Skip if pre-execution failed.
-      if (manager.disabled) {
-        log.info(`Skipping post-execution hook for ${actionName} due to earlier failure.`);
-        continue;
-      }
-
-      try {
-        await manager.runAsyncCallback("postExecution");
-        Uptake.reportAction(actionName, Uptake.ACTION_SUCCESS);
-      } catch (err) {
-        log.info(`Could not run post-execution hook for ${actionName}:`, err.message);
-        Uptake.reportAction(actionName, Uptake.ACTION_POST_EXECUTION_ERROR);
-      }
-    }
-
-    // Nuke sandboxes
-    Object.values(actionSandboxManagers).forEach(manager => manager.removeHold("recipeRunner"));
+    await actions.finalize();
 
     // Close storage connections
     await AddonStudies.close();
@@ -295,26 +234,6 @@ var RecipeRunner = {
     Uptake.reportRunner(Uptake.RUNNER_SUCCESS);
   },
 
-  async loadActionSandboxManagers() {
-    const actions = await NormandyApi.fetchActions();
-    const actionSandboxManagers = {};
-    for (const action of actions) {
-      try {
-        const implementation = await NormandyApi.fetchImplementation(action);
-        actionSandboxManagers[action.name] = new ActionSandboxManager(implementation);
-      } catch (err) {
-        log.warn(`Could not fetch implementation for ${action.name}:`, err);
-
-        let status = Uptake.ACTION_SERVER_ERROR;
-        if (/NetworkError/.test(err)) {
-          status = Uptake.ACTION_NETWORK_ERROR;
-        }
-        Uptake.reportAction(action.name, status);
-      }
-    }
-    return actionSandboxManagers;
-  },
-
   getFilterContext(recipe) {
     return {
       normandy: Object.assign(ClientEnvironment.getEnvironment(), {

toolkit/components/normandy/test/browser/browser.ini

@@ -8,9 +8,12 @@ head = head.js
 skip-if = !healthreport || !telemetry
 [browser_about_studies.js]
 skip-if = true # bug 1442712
+[browser_action_ConsoleLog.js]
 [browser_ActionSandboxManager.js]
+[browser_ActionsManager.js]
 [browser_Addons.js]
 [browser_AddonStudies.js]
+[browser_BaseAction.js]
 [browser_CleanupManager.js]
 [browser_ClientEnvironment.js]
 [browser_EventEmitter.js]
@@ -22,4 +25,4 @@ skip-if = true # bug 1442712
 [browser_PreferenceExperiments.js]
 [browser_RecipeRunner.js]
 [browser_ShieldPreferences.js]
-[browser_Storage.js]
+[browser_Storage.js]

toolkit/components/normandy/test/browser/browser_ActionsManager.js

@@ -0,0 +1,313 @@
+"use strict";
+
+ChromeUtils.import("resource://normandy/lib/ActionsManager.jsm", this);
+ChromeUtils.import("resource://normandy/lib/NormandyApi.jsm", this);
+ChromeUtils.import("resource://normandy/lib/Uptake.jsm", this);
+
+// It should only fetch implementations for actions that don't exist locally
+decorate_task(
+  withStub(NormandyApi, "fetchActions"),
+  withStub(NormandyApi, "fetchImplementation"),
+  async function(fetchActionsStub, fetchImplementationStub) {
+    const remoteAction = {name: "remote-action"};
+    const localAction = {name: "local-action"};
+    fetchActionsStub.resolves([remoteAction, localAction]);
+    fetchImplementationStub.callsFake(async () => "");
+
+    const manager = new ActionsManager();
+    manager.localActions = {"local-action": {}};
+    await manager.fetchRemoteActions();
+
+    is(fetchActionsStub.callCount, 1, "action metadata should be fetched");
+    Assert.deepEqual(
+      fetchImplementationStub.getCall(0).args,
+      [remoteAction],
+      "only the remote action's implementation should be fetched",
+    );
+  },
+);
+
+// Test life cycle methods for remote actions
+decorate_task(
+  withStub(Uptake, "reportAction"),
+  withStub(Uptake, "reportRecipe"),
+  async function(reportActionStub, reportRecipeStub) {
+    let manager = new ActionsManager();
+    const recipe = {id: 1, action: "test-remote-action-used"};
+
+    const sandboxManagerUsed = {
+      removeHold: sinon.stub(),
+      runAsyncCallback: sinon.stub(),
+    };
+    const sandboxManagerUnused = {
+      removeHold: sinon.stub(),
+      runAsyncCallback: sinon.stub(),
+    };
+    manager.remoteActionSandboxes = {
+      "test-remote-action-used": sandboxManagerUsed,
+      "test-remote-action-unused": sandboxManagerUnused
+    };
+    manager.localActions = {};
+
+    await manager.preExecution();
+    await manager.runRecipe(recipe);
+    await manager.finalize();
+
+    Assert.deepEqual(
+      sandboxManagerUsed.runAsyncCallback.args,
+      [
+        ["preExecution"],
+        ["action", recipe],
+        ["postExecution"],
+      ],
+      "The expected life cycle events should be called on the used sandbox action manager",
+    );
+    Assert.deepEqual(
+      sandboxManagerUnused.runAsyncCallback.args,
+      [
+        ["preExecution"],
+        ["postExecution"],
+      ],
+      "The expected life cycle events should be called on the unused sandbox action manager",
+    );
+    Assert.deepEqual(
+      sandboxManagerUsed.removeHold.args,
+      [["ActionsManager"]],
+      "ActionsManager should remove holds on the sandbox managers during finalize.",
+    );
+    Assert.deepEqual(
+      sandboxManagerUnused.removeHold.args,
+      [["ActionsManager"]],
+      "ActionsManager should remove holds on the sandbox managers during finalize.",
+    );
+
+    Assert.deepEqual(reportActionStub.args, [
+      ["test-remote-action-used", Uptake.ACTION_SUCCESS],
+      ["test-remote-action-unused", Uptake.ACTION_SUCCESS],
+    ]);
+    Assert.deepEqual(reportRecipeStub.args, [[recipe.id, Uptake.RECIPE_SUCCESS]]);
+  },
+);
+
+// Test life cycle for remote action that fails in pre-step
+decorate_task(
+  withStub(Uptake, "reportAction"),
+  withStub(Uptake, "reportRecipe"),
+  async function(reportActionStub, reportRecipeStub) {
+    let manager = new ActionsManager();
+    const recipe = {id: 1, action: "test-remote-action-broken"};
+
+    const sandboxManagerBroken = {
+      removeHold: sinon.stub(),
+      runAsyncCallback: sinon.stub().callsFake(callbackName => {
+        if (callbackName === "preExecution") {
+          throw new Error("mock preExecution failure");
+        }
+      }),
+    };
+    manager.remoteActionSandboxes = {
+      "test-remote-action-broken": sandboxManagerBroken,
+    };
+    manager.localActions = {};
+
+    await manager.preExecution();
+    await manager.runRecipe(recipe);
+    await manager.finalize();
+
+    Assert.deepEqual(
+      sandboxManagerBroken.runAsyncCallback.args,
+      [["preExecution"]],
+      "No async callbacks should be called after preExecution fails",
+    );
+    Assert.deepEqual(
+      sandboxManagerBroken.removeHold.args,
+      [["ActionsManager"]],
+      "sandbox holds should still be removed after a failure",
+    );
+
+    Assert.deepEqual(reportActionStub.args, [
+      ["test-remote-action-broken", Uptake.ACTION_PRE_EXECUTION_ERROR],
+    ]);
+    Assert.deepEqual(reportRecipeStub.args, [[recipe.id, Uptake.RECIPE_ACTION_DISABLED]]);
+  },
+);
+
+// Test life cycle for remote action that fails on a recipe-step
+decorate_task(
+  withStub(Uptake, "reportAction"),
+  withStub(Uptake, "reportRecipe"),
+  async function(reportActionStub, reportRecipeStub) {
+    let manager = new ActionsManager();
+    const recipe = {id: 1, action: "test-remote-action-broken"};
+
+    const sandboxManagerBroken = {
+      removeHold: sinon.stub(),
+      runAsyncCallback: sinon.stub().callsFake(callbackName => {
+        if (callbackName === "action") {
+          throw new Error("mock action failure");
+        }
+      }),
+    };
+    manager.remoteActionSandboxes = {
+      "test-remote-action-broken": sandboxManagerBroken,
+    };
+    manager.localActions = {};
+
+    await manager.preExecution();
+    await manager.runRecipe(recipe);
+    await manager.finalize();
+
+    Assert.deepEqual(
+      sandboxManagerBroken.runAsyncCallback.args,
+      [["preExecution"], ["action", recipe], ["postExecution"]],
+      "postExecution callback should still be called after action callback fails",
+    );
+    Assert.deepEqual(
+      sandboxManagerBroken.removeHold.args,
+      [["ActionsManager"]],
+      "sandbox holds should still be removed after a recipe failure",
+    );
+
+    Assert.deepEqual(reportActionStub.args, [["test-remote-action-broken", Uptake.ACTION_SUCCESS]]);
+    Assert.deepEqual(reportRecipeStub.args, [[recipe.id, Uptake.RECIPE_EXECUTION_ERROR]]);
+  },
+);
+
+// Test life cycle for remote action that fails in post-step
+decorate_task(
+  withStub(Uptake, "reportAction"),
+  withStub(Uptake, "reportRecipe"),
+  async function(reportActionStub, reportRecipeStub) {
+    let manager = new ActionsManager();
+    const recipe = {id: 1, action: "test-remote-action-broken"};
+
+    const sandboxManagerBroken = {
+      removeHold: sinon.stub(),
+      runAsyncCallback: sinon.stub().callsFake(callbackName => {
+        if (callbackName === "postExecution") {
+          throw new Error("mock postExecution failure");
+        }
+      }),
+    };
+    manager.remoteActionSandboxes = {
+      "test-remote-action-broken": sandboxManagerBroken,
+    };
+    manager.localActions = {};
+
+    await manager.preExecution();
+    await manager.runRecipe(recipe);
+    await manager.finalize();
+
+    Assert.deepEqual(
+      sandboxManagerBroken.runAsyncCallback.args,
+      [["preExecution"], ["action", recipe], ["postExecution"]],
+      "All callbacks should be executed",
+    );
+    Assert.deepEqual(
+      sandboxManagerBroken.removeHold.args,
+      [["ActionsManager"]],
+      "sandbox holds should still be removed after a failure",
+    );
+
+    Assert.deepEqual(reportRecipeStub.args, [[recipe.id, Uptake.RECIPE_SUCCESS]]);
+    Assert.deepEqual(reportActionStub.args, [
+      ["test-remote-action-broken", Uptake.ACTION_POST_EXECUTION_ERROR],
+    ]);
+  },
+);
+
+// Test life cycle methods for local actions
+decorate_task(
+  async function(reportActionStub, Stub) {
+    let manager = new ActionsManager();
+    const recipe = {id: 1, action: "test-local-action-used"};
+
+    let actionUsed = {
+      runRecipe: sinon.stub(),
+      finalize: sinon.stub(),
+    };
+    let actionUnused = {
+      runRecipe: sinon.stub(),
+      finalize: sinon.stub(),
+    };
+    manager.localActions = {
+      "test-local-action-used": actionUsed,
+      "test-local-action-unused": actionUnused,
+    };
+    manager.remoteActionSandboxes = {};
+
+    await manager.preExecution();
+    await manager.runRecipe(recipe);
+    await manager.finalize();
+
+    Assert.deepEqual(actionUsed.runRecipe.args, [[recipe]], "used action should be called with the recipe");
+    ok(actionUsed.finalize.calledOnce, "finalize should be called on used action");
+    Assert.deepEqual(actionUnused.runRecipe.args, [], "unused action should not be called with the recipe");
+    ok(actionUnused.finalize.calledOnce, "finalize should be called on the unused action");
+
+    // Uptake telemetry is handled by actions directly, so doesn't
+    // need to be tested for local action handling here.
+  },
+);
+
+// Likewise, error handling is dealt with internal to actions as well,
+// so doesn't need to be tested as a part of ActionsManager.
+
+// Test fetch remote actions
+decorate_task(
+  withStub(NormandyApi, "fetchActions"),
+  withStub(NormandyApi, "fetchImplementation"),
+  withStub(Uptake, "reportAction"),
+  async function(fetchActionsStub, fetchImplementationStub, reportActionStub) {
+    fetchActionsStub.callsFake(async () => [
+      {name: "remoteAction"},
+      {name: "missingImpl"},
+      {name: "migratedAction"},
+    ]);
+    fetchImplementationStub.callsFake(async ({ name }) => {
+      switch (name) {
+        case "remoteAction":
+          return "window.scriptRan = true";
+        case "missingImpl":
+          throw new Error(`Could not fetch implementation for ${name}: test error`);
+        case "migratedAction":
+          return "// this shouldn't be requested";
+        default:
+          throw new Error(`Could not fetch implementation for ${name}: unexpected action`);
+      }
+    });
+
+    const manager = new ActionsManager();
+    manager.localActions = {
+      migratedAction: {finalize: sinon.stub()},
+    };
+
+    await manager.fetchRemoteActions();
+
+    Assert.deepEqual(
+      Object.keys(manager.remoteActionSandboxes),
+      ["remoteAction"],
+      "remote action should have been loaded",
+    );
+
+    Assert.deepEqual(
+      fetchImplementationStub.args,
+      [[{name: "remoteAction"}], [{name: "missingImpl"}]],
+      "all remote actions should be requested",
+    );
+
+    Assert.deepEqual(
+      reportActionStub.args,
+      [["missingImpl", Uptake.ACTION_SERVER_ERROR]],
+      "Missing implementation should be reported via Uptake",
+    );
+
+    ok(
+      await manager.remoteActionSandboxes.remoteAction.evalInSandbox("window.scriptRan"),
+      "Implementations should be run in the sandbox",
+    );
+
+    // clean up sandboxes made by fetchRemoteActions
+    manager.finalize();
+  },
+);

toolkit/components/normandy/test/browser/browser_BaseAction.js

@@ -0,0 +1,204 @@
+"use strict";
+
+ChromeUtils.import("resource://normandy/actions/BaseAction.jsm", this);
+ChromeUtils.import("resource://normandy/lib/Uptake.jsm", this);
+
+class NoopAction extends BaseAction {
+  _run(recipe) {
+    // does nothing
+  }
+}
+
+class FailPreExecutionAction extends BaseAction {
+  constructor() {
+    super();
+    this._testRunFlag = false;
+    this._testFinalizeFlag = false;
+  }
+
+  _preExecution() {
+    throw new Error("Test error");
+  }
+
+  _run() {
+    this._testRunFlag = true;
+  }
+
+  _finalize() {
+    this._testFinalizeFlag = true;
+  }
+}
+
+class FailRunAction extends BaseAction {
+  constructor() {
+    super();
+    this._testRunFlag = false;
+    this._testFinalizeFlag = false;
+  }
+
+  _run(recipe) {
+    throw new Error("Test error");
+  }
+
+  _finalize() {
+    this._testFinalizeFlag = true;
+  }
+}
+
+class FailFinalizeAction extends BaseAction {
+  _run(recipe) {
+    // does nothing
+  }
+
+  _finalize() {
+    throw new Error("Test error");
+  }
+}
+
+let _recipeId = 1;
+function recipeFactory(overrides) {
+  let defaults = {
+    id: _recipeId++,
+    arguments: {},
+  };
+  Object.assign(defaults, overrides);
+  return defaults;
+}
+
+// Test that per-recipe uptake telemetry is recorded
+decorate_task(
+  withStub(Uptake, "reportRecipe"),
+  async function(reportRecipeStub) {
+    const action = new NoopAction();
+    const recipe = recipeFactory();
+    await action.runRecipe(recipe);
+    Assert.deepEqual(
+      reportRecipeStub.args,
+      [[recipe.id, Uptake.RECIPE_SUCCESS]],
+      "per-recipe uptake telemetry should be reported",
+    );
+  },
+);
+
+// Finalize causes action telemetry to be recorded
+decorate_task(
+  withStub(Uptake, "reportAction"),
+  async function(reportActionStub) {
+    const action = new NoopAction();
+    await action.finalize();
+    ok(action.finalized, "Action should be marked as finalized");
+    Assert.deepEqual(
+      reportActionStub.args,
+      [[action.name, Uptake.ACTION_SUCCESS]],
+      "action uptake telemetry should be reported",
+    );
+  },
+);
+
+// Recipes can't be run after finalize is called
+decorate_task(
+  withStub(Uptake, "reportRecipe"),
+  async function(reportRecipeStub) {
+    const action = new NoopAction();
+    const recipe1 = recipeFactory();
+    const recipe2 = recipeFactory();
+
+    await action.runRecipe(recipe1);
+    await action.finalize();
+
+    Assert.rejects(
+      action.runRecipe(recipe2),
+      /^Error: Action has already been finalized$/,
+      "running recipes after finalization is an error",
+    );
+
+    Assert.deepEqual(
+      reportRecipeStub.args,
+      [[recipe1.id, Uptake.RECIPE_SUCCESS]],
+      "Only recipes executed prior to finalizer should report uptake telemetry",
+    );
+  },
+);
+
+// Test an action with a failing pre-execution step
+decorate_task(
+  withStub(Uptake, "reportRecipe"),
+  withStub(Uptake, "reportAction"),
+  async function(reportRecipeStub, reportActionStub) {
+    const recipe = recipeFactory();
+    const action = new FailPreExecutionAction();
+    ok(action.failed, "Action should fail during pre-execution fail");
+
+    // Should not throw, even though the action is in a failed state.
+    await action.runRecipe(recipe);
+
+    // Should not throw, even though the action is in a failed state.
+    await action.finalize();
+
+    is(action._testRunFlag, false, "_run should not have been caled");
+    is(action._testFinalizeFlag, false, "_finalize should not have been caled");
+
+    Assert.deepEqual(
+      reportRecipeStub.args,
+      [[recipe.id, Uptake.RECIPE_ACTION_DISABLED]],
+      "Recipe should report recipe status as action disabled",
+    );
+
+    Assert.deepEqual(
+      reportActionStub.args,
+      [[action.name, Uptake.ACTION_PRE_EXECUTION_ERROR]],
+      "Action should report pre execution error",
+    );
+  },
+);
+
+// Test an action with a failing recipe step
+decorate_task(
+  withStub(Uptake, "reportRecipe"),
+  withStub(Uptake, "reportAction"),
+  async function(reportRecipeStub, reportActionStub) {
+    const recipe = recipeFactory();
+    const action = new FailRunAction();
+    await action.runRecipe(recipe);
+    await action.finalize();
+    ok(!action.failed, "Action should not be marked as failed due to a recipe failure");
+
+    ok(action._testFinalizeFlag, "_finalize should have been called");
+
+    Assert.deepEqual(
+      reportRecipeStub.args,
+      [[recipe.id, Uptake.RECIPE_EXECUTION_ERROR]],
+      "Recipe should report recipe execution error",
+    );
+
+    Assert.deepEqual(
+      reportActionStub.args,
+      [[action.name, Uptake.ACTION_SUCCESS]],
+      "Action should report success",
+    );
+  },
+);
+
+// Test an action with a failing finalize step
+decorate_task(
+  withStub(Uptake, "reportRecipe"),
+  withStub(Uptake, "reportAction"),
+  async function(reportRecipeStub, reportActionStub) {
+    const recipe = recipeFactory();
+    const action = new FailFinalizeAction();
+    await action.runRecipe(recipe);
+    await action.finalize();
+
+    Assert.deepEqual(
+      reportRecipeStub.args,
+      [[recipe.id, Uptake.RECIPE_SUCCESS]],
+      "Recipe should report success",
+    );
+
+    Assert.deepEqual(
+      reportActionStub.args,
+      [[action.name, Uptake.ACTION_POST_EXECUTION_ERROR]],
+      "Action should report post execution error",
+    );
+  },
+);

toolkit/components/normandy/test/browser/browser_RecipeRunner.js

@@ -6,6 +6,7 @@ ChromeUtils.import("resource://normandy/lib/ClientEnvironment.jsm", this);
 ChromeUtils.import("resource://normandy/lib/CleanupManager.jsm", this);
 ChromeUtils.import("resource://normandy/lib/NormandyApi.jsm", this);
 ChromeUtils.import("resource://normandy/lib/ActionSandboxManager.jsm", this);
+ChromeUtils.import("resource://normandy/lib/ActionsManager.jsm", this);
 ChromeUtils.import("resource://normandy/lib/AddonStudies.jsm", this);
 ChromeUtils.import("resource://normandy/lib/Uptake.jsm", this);
 
@@ -120,80 +121,46 @@ async function withMockActionSandboxManagers(actions, testFunction) {
 }
 
 decorate_task(
-  withMockNormandyApi,
   withSpy(AddonStudies, "close"),
   withStub(Uptake, "reportRunner"),
-  withStub(Uptake, "reportAction"),
-  withStub(Uptake, "reportRecipe"),
-  async function testRun(mockApi, closeSpy, reportRunner, reportAction, reportRecipe) {
-    const matchAction = {name: "matchAction"};
-    const noMatchAction = {name: "noMatchAction"};
-    mockApi.actions = [matchAction, noMatchAction];
-
+  withStub(NormandyApi, "fetchRecipes"),
+  withStub(ActionsManager.prototype, "fetchRemoteActions"),
+  withStub(ActionsManager.prototype, "preExecution"),
+  withStub(ActionsManager.prototype, "runRecipe"),
+  withStub(ActionsManager.prototype, "finalize"),
+  async function testRun(
+    closeSpy,
+    reportRunnerStub,
+    fetchRecipesStub,
+    fetchRemoteActionsStub,
+    preExecutionStub,
+    runRecipeStub,
+    finalizeStub
+  ) {
     const matchRecipe = {id: "match", action: "matchAction", filter_expression: "true"};
     const noMatchRecipe = {id: "noMatch", action: "noMatchAction", filter_expression: "false"};
     const missingRecipe = {id: "missing", action: "missingAction", filter_expression: "true"};
-    mockApi.recipes = [matchRecipe, noMatchRecipe, missingRecipe];
+    fetchRecipesStub.callsFake(async () => [matchRecipe, noMatchRecipe, missingRecipe]);
 
-    await withMockActionSandboxManagers(mockApi.actions, async managers => {
-      const matchManager = managers.matchAction;
-      const noMatchManager = managers.noMatchAction;
+    await RecipeRunner.run();
 
-      await RecipeRunner.run();
+    ok(fetchRemoteActionsStub.calledOnce, "remote actions should be fetched");
+    ok(preExecutionStub.calledOnce, "pre-execution hooks should be run");
+    Assert.deepEqual(
+      runRecipeStub.args,
+      [[matchRecipe], [missingRecipe]],
+      "recipe with matching filters should be executed",
+    );
 
-      // match should be called for preExecution, action, and postExecution
-      sinon.assert.calledWith(matchManager.runAsyncCallback, "preExecution");
-      sinon.assert.calledWith(matchManager.runAsyncCallback, "action", matchRecipe);
-      sinon.assert.calledWith(matchManager.runAsyncCallback, "postExecution");
-
-      // noMatch should be called for preExecution and postExecution, and skipped
-      // for action since the filter expression does not match.
-      sinon.assert.calledWith(noMatchManager.runAsyncCallback, "preExecution");
-      sinon.assert.neverCalledWith(noMatchManager.runAsyncCallback, "action", noMatchRecipe);
-      sinon.assert.calledWith(noMatchManager.runAsyncCallback, "postExecution");
-
-      // missing is never called at all due to no matching action/manager.
-
-      // Test uptake reporting
-      sinon.assert.calledWith(reportRunner, Uptake.RUNNER_SUCCESS);
-      sinon.assert.calledWith(reportAction, "matchAction", Uptake.ACTION_SUCCESS);
-      sinon.assert.calledWith(reportAction, "noMatchAction", Uptake.ACTION_SUCCESS);
-      sinon.assert.calledWith(reportRecipe, "match", Uptake.RECIPE_SUCCESS);
-      sinon.assert.neverCalledWith(reportRecipe, "noMatch", Uptake.RECIPE_SUCCESS);
-      sinon.assert.calledWith(reportRecipe, "missing", Uptake.RECIPE_INVALID_ACTION);
-    });
+    // Test uptake reporting
+    Assert.deepEqual(
+      reportRunnerStub.args,
+      [[Uptake.RUNNER_SUCCESS]],
+      "RecipeRunner should report uptake telemetry",
+    );
 
     // Ensure storage is closed after the run.
-    sinon.assert.calledOnce(closeSpy);
-  }
-);
-
-decorate_task(
-  withMockNormandyApi,
-  async function testRunRecipeError(mockApi) {
-    const reportRecipe = sinon.stub(Uptake, "reportRecipe");
-
-    const action = {name: "action"};
-    mockApi.actions = [action];
-
-    const recipe = {id: "recipe", action: "action", filter_expression: "true"};
-    mockApi.recipes = [recipe];
-
-    await withMockActionSandboxManagers(mockApi.actions, async managers => {
-      const manager = managers.action;
-      manager.runAsyncCallback.callsFake(async callbackName => {
-        if (callbackName === "action") {
-          throw new Error("Action execution failure");
-        }
-      });
-
-      await RecipeRunner.run();
-
-      // Uptake should report that the recipe threw an exception
-      sinon.assert.calledWith(reportRecipe, "recipe", Uptake.RECIPE_EXECUTION_ERROR);
-    });
-
-    reportRecipe.restore();
+    ok(closeSpy.calledOnce, "Storage should be closed after the run");
   }
 );
 
@@ -237,106 +204,6 @@ decorate_task(
   }
 );
 
-decorate_task(
-  withMockNormandyApi,
-  async function testRunPreExecutionFailure(mockApi) {
-    const closeSpy = sinon.spy(AddonStudies, "close");
-    const reportAction = sinon.stub(Uptake, "reportAction");
-    const reportRecipe = sinon.stub(Uptake, "reportRecipe");
-
-    const passAction = {name: "passAction"};
-    const failAction = {name: "failAction"};
-    mockApi.actions = [passAction, failAction];
-
-    const passRecipe = {id: "pass", action: "passAction", filter_expression: "true"};
-    const failRecipe = {id: "fail", action: "failAction", filter_expression: "true"};
-    mockApi.recipes = [passRecipe, failRecipe];
-
-    await withMockActionSandboxManagers(mockApi.actions, async managers => {
-      const passManager = managers.passAction;
-      const failManager = managers.failAction;
-      failManager.runAsyncCallback.returns(Promise.reject(new Error("oh no")));
-
-      await RecipeRunner.run();
-
-      // pass should be called for preExecution, action, and postExecution
-      sinon.assert.calledWith(passManager.runAsyncCallback, "preExecution");
-      sinon.assert.calledWith(passManager.runAsyncCallback, "action", passRecipe);
-      sinon.assert.calledWith(passManager.runAsyncCallback, "postExecution");
-
-      // fail should only be called for preExecution, since it fails during that
-      sinon.assert.calledWith(failManager.runAsyncCallback, "preExecution");
-      sinon.assert.neverCalledWith(failManager.runAsyncCallback, "action", failRecipe);
-      sinon.assert.neverCalledWith(failManager.runAsyncCallback, "postExecution");
-
-      sinon.assert.calledWith(reportAction, "passAction", Uptake.ACTION_SUCCESS);
-      sinon.assert.calledWith(reportAction, "failAction", Uptake.ACTION_PRE_EXECUTION_ERROR);
-      sinon.assert.calledWith(reportRecipe, "fail", Uptake.RECIPE_ACTION_DISABLED);
-    });
-
-    // Ensure storage is closed after the run, despite the failures.
-    sinon.assert.calledOnce(closeSpy);
-    closeSpy.restore();
-    reportAction.restore();
-    reportRecipe.restore();
-  }
-);
-
-decorate_task(
-  withMockNormandyApi,
-  async function testRunPostExecutionFailure(mockApi) {
-    const reportAction = sinon.stub(Uptake, "reportAction");
-
-    const failAction = {name: "failAction"};
-    mockApi.actions = [failAction];
-
-    const failRecipe = {action: "failAction", filter_expression: "true"};
-    mockApi.recipes = [failRecipe];
-
-    await withMockActionSandboxManagers(mockApi.actions, async managers => {
-      const failManager = managers.failAction;
-      failManager.runAsyncCallback.callsFake(async callbackName => {
-        if (callbackName === "postExecution") {
-          throw new Error("postExecution failure");
-        }
-      });
-
-      await RecipeRunner.run();
-
-      // fail should be called for every stage
-      sinon.assert.calledWith(failManager.runAsyncCallback, "preExecution");
-      sinon.assert.calledWith(failManager.runAsyncCallback, "action", failRecipe);
-      sinon.assert.calledWith(failManager.runAsyncCallback, "postExecution");
-
-      // Uptake should report a post-execution error
-      sinon.assert.calledWith(reportAction, "failAction", Uptake.ACTION_POST_EXECUTION_ERROR);
-    });
-
-    reportAction.restore();
-  }
-);
-
-decorate_task(
-  withMockNormandyApi,
-  async function testLoadActionSandboxManagers(mockApi) {
-    mockApi.actions = [
-      {name: "normalAction"},
-      {name: "missingImpl"},
-    ];
-    mockApi.implementations.normalAction = "window.scriptRan = true";
-
-    const managers = await RecipeRunner.loadActionSandboxManagers();
-    ok("normalAction" in managers, "Actions with implementations have managers");
-    ok(!("missingImpl" in managers), "Actions without implementations are skipped");
-
-    const normalManager = managers.normalAction;
-    ok(
-      await normalManager.evalInSandbox("window.scriptRan"),
-      "Implementations are run in the sandbox",
-    );
-  }
-);
-
 // test init() in dev mode
 decorate_task(
   withPrefEnv({

toolkit/components/normandy/test/browser/browser_action_ConsoleLog.js

@@ -0,0 +1,45 @@
+"use strict";
+
+ChromeUtils.import("resource://normandy/actions/ConsoleLog.jsm", this);
+ChromeUtils.import("resource://normandy/lib/Uptake.jsm", this);
+
+// Test that logging works
+add_task(async function logging_works() {
+  const action = new ConsoleLog();
+  const infoStub = sinon.stub(action.log, "info");
+  try {
+    const recipe = {id: 1, arguments: {message: "Hello, world!"}};
+    await action.runRecipe(recipe);
+    Assert.deepEqual(infoStub.args, ["Hello, world!"], "the message should be logged");
+  } finally {
+    infoStub.restore();
+  }
+});
+
+
+// test that argument validation works
+decorate_task(
+  withStub(Uptake, "reportRecipe"),
+  async function arguments_are_validated(reportRecipeStub) {
+    const action = new ConsoleLog();
+    const infoStub = sinon.stub(action.log, "info");
+
+    try {
+      // message is required
+      let recipe = {id: 1, arguments: {}};
+      await action.runRecipe(recipe);
+      Assert.deepEqual(infoStub.args, [], "no message should be logged");
+      Assert.deepEqual(reportRecipeStub.args, [[recipe.id, Uptake.RECIPE_EXECUTION_ERROR]]);
+
+      reportRecipeStub.reset();
+
+      // message must be a string
+      recipe = {id: 1, arguments: {message: 1}};
+      await action.runRecipe(recipe);
+      Assert.deepEqual(infoStub.args, [], "no message should be logged");
+      Assert.deepEqual(reportRecipeStub.args, [[recipe.id, Uptake.RECIPE_EXECUTION_ERROR]]);
+    } finally {
+      infoStub.restore();
+    }
+  },
+);

toolkit/components/normandy/test/browser/head.js

@@ -129,7 +129,7 @@ this.withMockNormandyApi = function(testFunction) {
       async action => {
         const impl = mockApi.implementations[action.name];
         if (!impl) {
-          throw new Error("Missing");
+          throw new Error(`Missing implementation for ${action.name}`);
         }
         return impl;
       }