From 8ce97d69ebe1a464fc3b1d137cb6b1e38b0c26b6 Mon Sep 17 00:00:00 2001
From: Will Hawkins <whh8b@obs.cr>
Date: Thu, 10 Oct 2019 14:23:12 -0400
Subject: [PATCH] Fix Issue #43: Handle failures when running privileged
 scripts in Chrome

Move catch blocks down a level to make sure that an individual
script's failure does not mean that an entire category is missed.
---
 lib/core/seleniumRunner.js | 69 ++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/lib/core/seleniumRunner.js b/lib/core/seleniumRunner.js
index 70e67af2d..41ccd7e42 100644
--- a/lib/core/seleniumRunner.js
+++ b/lib/core/seleniumRunner.js
@@ -440,6 +440,7 @@ class SeleniumRunner {
   }
 
   /**
+   * Run a single script from a particular category.
    *
    * Scripts should be valid statements or IIFEs '(function() {...})()' that can run
    * on their own in the browser console. Prepend with 'return' to return result of statement to Browsertime.
@@ -502,8 +503,9 @@ class SeleniumRunner {
   }
 
   /**
-   * Run scripts by category.
-   * @param {*} scriptsByCategory
+   * Run all the scripts
+   * @param {*} scriptsByCategory - dictionary whose keys are the category
+   * names and whose values are dictionaries of scripts for that category.
    * @param {boolean} isAsync - is the script synchrously or async?
    */
   async runScripts(scriptsByCategory, isAsync) {
@@ -511,41 +513,25 @@ class SeleniumRunner {
     const results = {};
     for (let categoryName of categoryNames) {
       const category = scriptsByCategory[categoryName];
-      try {
-        results[categoryName] = await this.runScriptInCategory(
-          category,
-          isAsync
-        );
-      } catch (e) {
-        if (e.extra.cause === 'PrivilegeError') {
-          // Ignore those scripts that fail to execute because they
-          // wanted privileges that the browser cannot provide.
-          log.verbose(
-            'Did not have enough privileges to execute user script: ' +
-              e +
-              '; ignoring.'
-          );
-        } else {
-          log.error('Failed to execute user script: ' + e);
-          results[categoryName] = undefined;
-        }
-      }
+      results[categoryName] = await this.runScriptInCategory(category, isAsync);
     }
     return results;
   }
 
   /**
-   * Run scripts in category.
-   * @param {*} category
+   * Run all the scripts in a category.
+   * @param {*} category - a dictionary whose keys are script names and
+   * whose values are scripts themselves.
    * @param {boolean} isAsync - is the script synchrously or async?
    */
   async runScriptInCategory(category, isAsync) {
     const scriptNames = Object.keys(category);
     const results = {};
-    let requires = [];
 
     for (let scriptName of scriptNames) {
+      let requires = [];
       let isAsyncOverride = false;
+      let result = null;
       let script = category[scriptName].content;
       // Assume that if the string in content is null
       // the function is not null and we want to run
@@ -553,9 +539,12 @@ class SeleniumRunner {
       if (!script) {
         let func = category[scriptName].function;
         if (!func) {
-          throw 'Function and script cannot both be null in ' +
-            scriptName +
-            '.';
+          log.error(
+            'Function and script cannot both be null in ' +
+              scriptName +
+              '; skipping.'
+          );
+          continue;
         }
         // We wrap the source code of the function in parenthesis
         // "(...)" to contain it in a separate scope. We add a
@@ -568,12 +557,26 @@ class SeleniumRunner {
         isAsyncOverride = category[scriptName].isAsync;
       }
 
-      const result = await this.runScriptFromCategory(
-        script,
-        isAsync || isAsyncOverride,
-        scriptName,
-        requires
-      );
+      try {
+        result = await this.runScriptFromCategory(
+          script,
+          isAsync || isAsyncOverride,
+          scriptName,
+          requires
+        );
+      } catch (e) {
+        if (e.extra.cause === 'PrivilegeError') {
+          // Ignore those scripts that fail to execute because they
+          // wanted privileges that the browser cannot provide.
+          log.verbose(
+            'Did not have enough privileges to execute user script: ' +
+              e +
+              '; ignoring.'
+          );
+        } else {
+          log.error('Failed to execute user script: ' + e);
+        }
+      }
       if (!(result === null || result === undefined)) {
         results[scriptName] = result;
       }