core: remove recoverOrThrow / err.fatal

lighthouse-core/gather/gather-runner.js

@@ -130,20 +130,6 @@ class GatherRunner {
     });
   }
 
-  /**
-   * Test any error output from the promise, absorbing non-fatal errors and
-   * throwing on fatal ones so that run is stopped.
-   * @param {Promise<*>} promise
-   * @return {Promise<void>}
-   */
-  static recoverOrThrow(promise) {
-    return promise.catch(err => {
-      if (err.fatal) {
-        throw err;
-      }
-    });
-  }
-
   /**
    * Returns an error if the original network request failed or wasn't found.
    * @param {string} url The URL of the original requested page.
@@ -197,7 +183,6 @@ class GatherRunner {
       passContext.options = gathererDefn.options || {};
       const artifactPromise = Promise.resolve().then(_ => gatherer.beforePass(passContext));
       gathererResults[gatherer.name] = [artifactPromise];
-      await GatherRunner.recoverOrThrow(artifactPromise);
     }
   }
 
@@ -241,7 +226,6 @@ class GatherRunner {
       const gathererResult = gathererResults[gatherer.name] || [];
       gathererResult.push(artifactPromise);
       gathererResults[gatherer.name] = gathererResult;
-      await GatherRunner.recoverOrThrow(artifactPromise);
     }
   }
 
@@ -308,7 +292,6 @@ class GatherRunner {
       const gathererResult = gathererResults[gatherer.name] || [];
       gathererResult.push(artifactPromise);
       gathererResults[gatherer.name] = gathererResult;
-      await GatherRunner.recoverOrThrow(artifactPromise);
       log.verbose('statusEnd', status);
     }
 
@@ -319,7 +302,7 @@ class GatherRunner {
   /**
    * Takes the results of each gatherer phase for each gatherer and uses the
    * last produced value (that's not undefined) as the artifact for that
-   * gatherer. If a non-fatal error was rejected from a gatherer phase,
+   * gatherer. If an error was rejected from a gatherer phase,
    * uses that error object as the artifact instead.
    * @param {Partial<GathererResults>} gathererResults
    * @param {LH.BaseArtifacts} baseArtifacts
@@ -341,8 +324,7 @@ class GatherRunner {
         // Typecast pretends artifact always provided here, but checked below for top-level `throw`.
         gathererArtifacts[gathererName] = /** @type {NonVoid<PhaseResult>} */ (artifact);
       } catch (err) {
-        // An error result must be non-fatal to not have caused an exit by now,
-        // so return it to runner to handle turning it into an error audit.
+        // return error runner to handle turning it into an error audit.
         gathererArtifacts[gathererName] = err;
       }
 

lighthouse-core/gather/gatherers/gatherer.js

@@ -13,9 +13,8 @@
  * method. All methods can return the artifact value directly or return a
  * Promise that resolves to that value.
  *
- * If an Error is thrown (or a Promise that rejects on an Error), the
- * GatherRunner will check for a `fatal` property on the Error. If not set to
- * `true`, the runner will treat it as an error internal to the gatherer and
+ * If an Error is thrown (or a Promise that rejects on an Error),
+ * the runner will treat it as an error internal to the gatherer and
  * continue execution of any remaining gatherers.
  */
 class Gatherer {

lighthouse-core/runner.js

@@ -251,8 +251,7 @@ class Runner {
           throw new Error(`Required ${artifactName} gatherer did not run.`);
         }
 
-        // If artifact was an error, it must be non-fatal (or gatherRunner would
-        // have thrown). Output error result on behalf of audit.
+        // If artifact was an error, output error result on behalf of audit.
         if (artifacts[artifactName] instanceof Error) {
           /** @type {Error} */
           // @ts-ignore An artifact *could* be an Error, but caught here, so ignore elsewhere.
@@ -286,12 +285,9 @@ class Runner {
       auditResult = Audit.generateAuditResult(audit, product);
     } catch (err) {
       log.warn(audit.meta.id, `Caught exception: ${err.message}`);
-      if (err.fatal) {
-        throw err;
-      }
 
       Sentry.captureException(err, {tags: {audit: audit.meta.id}, level: 'error'});
-      // Non-fatal error become error audit result.
+      // Errors become error audit result.
       const errorMessage = err.friendlyMessage ?
         `${err.friendlyMessage} (${err.message})` :
         `Audit error: ${err.message}`;

lighthouse-core/test/gather/gather-runner-test.js

@@ -915,7 +915,7 @@ describe('GatherRunner', function() {
       });
     });
 
-    it('supports sync and async throwing of non-fatal errors from gatherers', () => {
+    it('supports sync and async throwing of errors from gatherers', () => {
       const gatherers = [
         // sync
         new class BeforeSync extends Gatherer {
@@ -974,38 +974,6 @@ describe('GatherRunner', function() {
       });
     });
 
-    it('rejects if a gatherer returns a fatal error', () => {
-      const errorMessage = 'Gather Failed in pass()';
-      const err = new Error(errorMessage);
-      err.fatal = true;
-      const gatherers = [
-        // sync
-        new class GathererSuccess extends Gatherer {
-          afterPass() {
-            return 1;
-          }
-        }(),
-        new class GathererFailure extends Gatherer {
-          pass() {
-            return Promise.reject(err);
-          }
-        },
-      ].map(instance => ({instance}));
-      const passes = [{
-        blankDuration: 0,
-        gatherers,
-      }];
-
-      return GatherRunner.run(passes, {
-        driver: fakeDriver,
-        requestedUrl: 'https://example.com',
-        settings: {},
-        config: new Config({}),
-      }).then(
-        _ => assert.ok(false),
-        err => assert.strictEqual(err.message, errorMessage));
-    });
-
     it('rejects if a gatherer does not provide an artifact', () => {
       const passes = [{
         blankDuration: 0,

lighthouse-core/test/runner-test.js

@@ -290,7 +290,7 @@ describe('Runner', () => {
 
     // TODO: need to support save/load of artifact errors.
     // See https://github.com/GoogleChrome/lighthouse/issues/4984
-    it.skip('outputs an error audit result when required artifact was a non-fatal Error', () => {
+    it.skip('outputs an error audit result when required artifact was a Error', () => {
       const errorMessage = 'blurst of times';
       const artifactError = new Error(errorMessage);
 
@@ -326,7 +326,7 @@ describe('Runner', () => {
       requiredArtifacts: [],
     };
 
-    it('produces an error audit result when an audit throws a non-fatal Error', () => {
+    it('produces an error audit result when an audit throws an Error', () => {
       const errorMessage = 'Audit yourself';
       const config = new Config({
         settings: {
@@ -351,31 +351,6 @@ describe('Runner', () => {
         assert.ok(auditResult.errorMessage.includes(errorMessage));
       });
     });
-
-    it('rejects if an audit throws a fatal error', () => {
-      const errorMessage = 'Uh oh';
-      const config = new Config({
-        settings: {
-          auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
-        },
-        audits: [
-          class FatalThrowyAudit extends Audit {
-            static get meta() {
-              return testAuditMeta;
-            }
-            static audit() {
-              const fatalError = new Error(errorMessage);
-              fatalError.fatal = true;
-              throw fatalError;
-            }
-          },
-        ],
-      });
-
-      return Runner.run({}, {config}).then(
-        _ => assert.ok(false),
-        err => assert.strictEqual(err.message, errorMessage));
-    });
   });
 
   it('accepts devtoolsLog in artifacts', () => {