diff --git a/packages/core/core/src/Parcel.js b/packages/core/core/src/Parcel.js index 4b775a939a5..9b083bf8013 100644 --- a/packages/core/core/src/Parcel.js +++ b/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) { diff --git a/packages/core/core/src/ReporterRunner.js b/packages/core/core/src/ReporterRunner.js index 8545b432e96..03055e226a0 100644 --- a/packages/core/core/src/ReporterRunner.js +++ b/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,8 +23,8 @@ import BundleGraph from './BundleGraph'; import {tracer, PluginTracer} from '@parcel/profiler'; type Opts = {| - config: ParcelConfig, options: ParcelOptions, + reporters: Array>, workerFarm: WorkerFarm, |}; @@ -33,14 +32,15 @@ const instances: Set = new Set(); export default class ReporterRunner { workerFarm: WorkerFarm; - config: ParcelConfig; + errors: Error[]; options: ParcelOptions; pluginOptions: PluginOptions; reporters: Array>; 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,40 +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); - process.exitCode = 1; - } finally { - measurement && measurement.end(); } + + this.errors.push(reportError); + } finally { + measurement && measurement.end(); } - } catch (err) { - INTERNAL_ORIGINAL_CONSOLE.error(err); } } diff --git a/packages/core/integration-tests/test/integration/reporters-load-failure/.parcelrc b/packages/core/integration-tests/test/integration/reporters-load-failure/.parcelrc new file mode 100644 index 00000000000..fb4bd08065d --- /dev/null +++ b/packages/core/integration-tests/test/integration/reporters-load-failure/.parcelrc @@ -0,0 +1,4 @@ +{ + "extends": "@parcel/config-default", + "reporters": ["./test-reporter"] +} diff --git a/packages/core/integration-tests/test/integration/reporters-load-failure/index.js b/packages/core/integration-tests/test/integration/reporters-load-failure/index.js new file mode 100644 index 00000000000..f0ed26b348f --- /dev/null +++ b/packages/core/integration-tests/test/integration/reporters-load-failure/index.js @@ -0,0 +1 @@ +export function main() {} diff --git a/packages/core/integration-tests/test/integration/reporters-load-failure/yarn.lock b/packages/core/integration-tests/test/integration/reporters-load-failure/yarn.lock new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/core/integration-tests/test/reporters.js b/packages/core/integration-tests/test/reporters.js index e2acdcbd3bd..30cebe31864 100644 --- a/packages/core/integration-tests/test/reporters.js +++ b/packages/core/integration-tests/test/reporters.js @@ -4,9 +4,7 @@ import assert from 'assert'; import {execSync} from 'child_process'; import path from 'path'; -import {INTERNAL_ORIGINAL_CONSOLE} from '@parcel/logger'; -import {bundle} from '@parcel/test-utils'; -import sinon from 'sinon'; +import {bundler} from '@parcel/test-utils'; describe('reporters', () => { let successfulEntry = path.join( @@ -16,7 +14,14 @@ describe('reporters', () => { 'index.js', ); - let failingEntry = path.join( + let loadReporterFailureEntry = path.join( + __dirname, + 'integration', + 'reporters-load-failure', + 'index.js', + ); + + let failingReporterEntry = path.join( __dirname, 'integration', 'reporters-failure', @@ -32,42 +37,56 @@ describe('reporters', () => { ); }); - it('exit with an error code when an error is emitted', () => { + it('exit with an error code when a reporter fails to load', () => { assert.throws(() => - execSync(`parcel build --no-cache ${failingEntry}`, {stdio: 'ignore'}), + 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', () => { - let consoleError; - let processExitCode; + it('resolves when no errors are emitted', async () => { + let buildEvent = await bundler(successfulEntry).run(); - beforeEach(() => { - processExitCode = process.exitCode; - consoleError = sinon.stub(INTERNAL_ORIGINAL_CONSOLE, 'error'); + assert.equal(buildEvent.type, 'buildSuccess'); }); - afterEach(() => { - process.exitCode = processExitCode; - sinon.restore(); - }); - - it('exit successfully when no errors are emitted', async () => { - await bundle(successfulEntry); + it('rejects when a reporter fails to load', async () => { + try { + let buildEvent = await bundler(loadReporterFailureEntry).run(); - assert(!process.exitCode); + 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('exit with an error code when an error is emitted', async () => { - await bundle(failingEntry); + it('rejects when a reporter emits an error', async () => { + try { + let buildEvent = await bundler(failingReporterEntry).run(); - assert.equal(process.exitCode, 1); - assert( - consoleError.calledWithMatch({ - message: 'Failed to report buildSuccess', - }), - ); + throw new Error(buildEvent); + } catch (err) { + assert.equal(err.name, 'BuildError'); + assert.deepEqual( + err.diagnostics.map(d => d.message), + ['Failed to report buildSuccess'], + ); + } }); }); }); diff --git a/packages/core/parcel/src/cli.js b/packages/core/parcel/src/cli.js index d163b8e6d3d..e5ba8ef7c3f 100755 --- a/packages/core/parcel/src/cli.js +++ b/packages/core/parcel/src/cli.js @@ -404,7 +404,7 @@ async function run( await exit(1); } - await exit(process.exitCode); + await exit(); } }