Exit process with error code when reporters fail (#9735)

* Exit process when reporters fail

* Add cli handling

* Propagate reporter errors

Author: MonicaOlejniczak

packages/core/core/src/Parcel.js

@@ -152,8 +152,8 @@ export default class Parcel {
     this.#disposable.add(() => this.#watchEvents.dispose());
 
     this.#reporterRunner = new ReporterRunner({
-      config: this.#config,
       options: resolvedOptions,
+      reporters: await this.#config.getReporters(),
       workerFarm: this.#farm,
     });
     this.#disposable.add(this.#reporterRunner);
@@ -313,7 +313,7 @@ export default class Parcel {
       if (options.shouldTrace) {
         tracer.enable();
       }
-      this.#reporterRunner.report({
+      await this.#reporterRunner.report({
         type: 'buildStart',
       });
 
@@ -401,6 +401,11 @@ export default class Parcel {
         createValidationRequest({optionsRef: this.#optionsRef, assetRequests}),
         {force: assetRequests.length > 0},
       );
+
+      if (this.#reporterRunner.errors.length) {
+        throw this.#reporterRunner.errors;
+      }
+
       return event;
     } catch (e) {
       if (e instanceof BuildAbortError) {

packages/core/core/src/ReporterRunner.js

@@ -12,7 +12,6 @@ import {
   NamedBundle,
 } from './public/Bundle';
 import WorkerFarm, {bus} from '@parcel/workers';
-import ParcelConfig from './ParcelConfig';
 import logger, {
   patchConsole,
   unpatchConsole,
@@ -24,23 +23,24 @@ import BundleGraph from './BundleGraph';
 import {tracer, PluginTracer} from '@parcel/profiler';
 
 type Opts = {|
-  config: ParcelConfig,
   options: ParcelOptions,
+  reporters: Array<LoadedPlugin<Reporter>>,
   workerFarm: WorkerFarm,
 |};
 
 const instances: Set<ReporterRunner> = new Set();
 
 export default class ReporterRunner {
   workerFarm: WorkerFarm;
-  config: ParcelConfig;
+  errors: Error[];
   options: ParcelOptions;
   pluginOptions: PluginOptions;
   reporters: Array<LoadedPlugin<Reporter>>;
 
   constructor(opts: Opts) {
-    this.config = opts.config;
+    this.errors = [];
     this.options = opts.options;
+    this.reporters = opts.reporters;
     this.workerFarm = opts.workerFarm;
     this.pluginOptions = new PluginOptions(this.options);
 
@@ -86,39 +86,33 @@ export default class ReporterRunner {
   };
 
   async report(event: ReporterEvent) {
-    // We should catch all errors originating from reporter plugins to prevent infinite loops
-    try {
-      let reporters = this.reporters;
-      if (!reporters) {
-        this.reporters = await this.config.getReporters();
-        reporters = this.reporters;
-      }
-
-      for (let reporter of this.reporters) {
-        let measurement;
-        try {
-          // To avoid an infinite loop we don't measure trace events, as they'll
-          // result in another trace!
-          if (event.type !== 'trace') {
-            measurement = tracer.createMeasurement(reporter.name, 'reporter');
-          }
-          await reporter.plugin.report({
-            event,
-            options: this.pluginOptions,
-            logger: new PluginLogger({origin: reporter.name}),
-            tracer: new PluginTracer({
-              origin: reporter.name,
-              category: 'reporter',
-            }),
-          });
-        } catch (reportError) {
+    for (let reporter of this.reporters) {
+      let measurement;
+      try {
+        // To avoid an infinite loop we don't measure trace events, as they'll
+        // result in another trace!
+        if (event.type !== 'trace') {
+          measurement = tracer.createMeasurement(reporter.name, 'reporter');
+        }
+        await reporter.plugin.report({
+          event,
+          options: this.pluginOptions,
+          logger: new PluginLogger({origin: reporter.name}),
+          tracer: new PluginTracer({
+            origin: reporter.name,
+            category: 'reporter',
+          }),
+        });
+      } catch (reportError) {
+        if (event.type !== 'buildSuccess') {
+          // This will be captured by consumers
           INTERNAL_ORIGINAL_CONSOLE.error(reportError);
-        } finally {
-          measurement && measurement.end();
         }
+
+        this.errors.push(reportError);
+      } finally {
+        measurement && measurement.end();
       }
-    } catch (err) {
-      INTERNAL_ORIGINAL_CONSOLE.error(err);
     }
   }
 

packages/core/integration-tests/test/integration/reporters-failure/.parcelrc

@@ -0,0 +1,4 @@
+{
+  "extends": "@parcel/config-default",
+  "reporters": ["./test-reporter"]
+}

packages/core/integration-tests/test/integration/reporters-failure/index.js

@@ -0,0 +1 @@
+export function main() {}

packages/core/integration-tests/test/integration/reporters-failure/test-reporter/index.js

@@ -0,0 +1,9 @@
+const { Reporter } = require('@parcel/plugin');
+
+module.exports = new Reporter({
+  async report({ event }) {
+    if (event.type === 'buildSuccess') {
+      throw new Error('Failed to report buildSuccess');
+    }
+  }
+});

packages/core/integration-tests/test/integration/reporters-failure/test-reporter/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "test-reporter",
+  "version": "1.0.0",
+  "main": "index.js"
+}

packages/core/integration-tests/test/integration/reporters-failure/yarn.lock

@@ -0,0 +1,4 @@
+{
+  "extends": "@parcel/config-default",
+  "reporters": ["./test-reporter"]
+}

packages/core/integration-tests/test/integration/reporters-load-failure/.parcelrc

@@ -0,0 +1 @@
+export function main() {}

packages/core/integration-tests/test/integration/reporters-load-failure/index.js

@@ -0,0 +1,4 @@
+{
+  "extends": "@parcel/config-default",
+  "reporters": ["./test-reporter"]
+}

packages/core/integration-tests/test/integration/reporters-load-failure/yarn.lock

@@ -0,0 +1 @@
+export function main() {}

packages/core/integration-tests/test/integration/reporters-success/.parcelrc

@@ -0,0 +1,5 @@
+const { Reporter } = require('@parcel/plugin');
+
+module.exports = new Reporter({
+  async report({ event }) {}
+});

packages/core/integration-tests/test/integration/reporters-success/index.js

@@ -0,0 +1,5 @@
+{
+  "name": "test-reporter",
+  "version": "1.0.0",
+  "main": "index.js"
+}

packages/core/integration-tests/test/integration/reporters-success/test-reporter/index.js

@@ -0,0 +1,92 @@
+// @flow
+
+import assert from 'assert';
+import {execSync} from 'child_process';
+import path from 'path';
+
+import {bundler} from '@parcel/test-utils';
+
+describe('reporters', () => {
+  let successfulEntry = path.join(
+    __dirname,
+    'integration',
+    'reporters-success',
+    'index.js',
+  );
+
+  let loadReporterFailureEntry = path.join(
+    __dirname,
+    'integration',
+    'reporters-load-failure',
+    'index.js',
+  );
+
+  let failingReporterEntry = path.join(
+    __dirname,
+    'integration',
+    'reporters-failure',
+    'index.js',
+  );
+
+  describe('running on the cli', () => {
+    it('exit successfully when no errors are emitted', () => {
+      assert.doesNotThrow(() =>
+        execSync(`parcel build --no-cache ${successfulEntry}`, {
+          stdio: 'ignore',
+        }),
+      );
+    });
+
+    it('exit with an error code when a reporter fails to load', () => {
+      assert.throws(() =>
+        execSync(`parcel build --no-cache ${loadReporterFailureEntry}`, {
+          stdio: 'ignore',
+        }),
+      );
+    });
+
+    it('exit with an error code when a reporter emits an error', () => {
+      assert.throws(() =>
+        execSync(`parcel build --no-cache ${failingReporterEntry}`, {
+          stdio: 'ignore',
+        }),
+      );
+    });
+  });
+
+  describe('running on the programmatic api', () => {
+    it('resolves when no errors are emitted', async () => {
+      let buildEvent = await bundler(successfulEntry).run();
+
+      assert.equal(buildEvent.type, 'buildSuccess');
+    });
+
+    it('rejects when a reporter fails to load', async () => {
+      try {
+        let buildEvent = await bundler(loadReporterFailureEntry).run();
+
+        throw new Error(buildEvent);
+      } catch (err) {
+        assert.equal(err.name, 'Error');
+        assert.deepEqual(
+          err.diagnostics.map(d => d.message),
+          ['Cannot find Parcel plugin "./test-reporter"'],
+        );
+      }
+    });
+
+    it('rejects when a reporter emits an error', async () => {
+      try {
+        let buildEvent = await bundler(failingReporterEntry).run();
+
+        throw new Error(buildEvent);
+      } catch (err) {
+        assert.equal(err.name, 'BuildError');
+        assert.deepEqual(
+          err.diagnostics.map(d => d.message),
+          ['Failed to report buildSuccess'],
+        );
+      }
+    });
+  });
+});