From 8d8161bdc3cbf02416b617f3b43318e504ec6b04 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 10 Jan 2022 22:27:57 +0100 Subject: [PATCH 1/4] Add test --- packages/core/integration-tests/test/cache.js | 43 +++++++++++++++++++ .../parcel-transformer-included/index.js | 3 ++ 2 files changed, 46 insertions(+) diff --git a/packages/core/integration-tests/test/cache.js b/packages/core/integration-tests/test/cache.js index da809e78bb2..806251e35fe 100644 --- a/packages/core/integration-tests/test/cache.js +++ b/packages/core/integration-tests/test/cache.js @@ -5836,6 +5836,49 @@ describe('cache', function () { } }); + it('properly watches included files after a transformer error', async function () { + this.timeout(15000); + let subscription; + let fixture = path.join(__dirname, '/integration/included-file'); + try { + let b = bundler(path.join(fixture, 'index.txt'), { + inputFS: overlayFS, + shouldDisableCache: false, + }); + await overlayFS.mkdirp(fixture); + await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'a'); + subscription = await b.watch(); + let event = await getNextBuild(b); + invariant(event.type === 'buildSuccess'); + let output1 = await overlayFS.readFile( + event.bundleGraph.getBundles()[0].filePath, + 'utf8', + ); + assert.strictEqual(output1, 'a'); + + // Change included file + await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'ERROR'); + event = await getNextBuild(b); + invariant(event.type === 'buildFailure'); + assert.strictEqual(event.diagnostics[0].message, 'Custom error'); + + // Change included file back + await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'b'); + event = await getNextBuild(b); + invariant(event.type === 'buildSuccess'); + let output3 = await overlayFS.readFile( + event.bundleGraph.getBundles()[0].filePath, + 'utf8', + ); + assert.strictEqual(output3, 'b'); + } finally { + if (subscription) { + await subscription.unsubscribe(); + subscription = null; + } + } + }); + it('should support moving the project root', async function () { // This test relies on the real filesystem because the memory fs doesn't support renames. // But renameSync is broken on windows in CI with EPERM errors. Just skip this test for now. diff --git a/packages/core/integration-tests/test/integration/included-file/node_modules/parcel-transformer-included/index.js b/packages/core/integration-tests/test/integration/included-file/node_modules/parcel-transformer-included/index.js index e7f12e743cb..31412ca7b7c 100644 --- a/packages/core/integration-tests/test/integration/included-file/node_modules/parcel-transformer-included/index.js +++ b/packages/core/integration-tests/test/integration/included-file/node_modules/parcel-transformer-included/index.js @@ -6,6 +6,9 @@ module.exports = new Transformer({ let file = path.join(path.dirname(asset.filePath), "included.txt"); asset.invalidateOnFileChange(file); let content = await options.inputFS.readFile(file, "utf8"); + if (content === "ERROR") { + throw new Error("Custom error"); + } asset.setCode(content); return [asset]; } From 1dd65a748e222ed715f6a26369b79f73e486f17e Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 10 Jan 2022 22:28:16 +0100 Subject: [PATCH 2/4] Catch runPipelines error to retain invalidations --- packages/core/core/src/Transformation.js | 15 +++++++++++---- packages/core/core/src/requests/AssetRequest.js | 13 ++++++++++--- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/core/core/src/Transformation.js b/packages/core/core/src/Transformation.js index 321a327e315..33bdfe0ebda 100644 --- a/packages/core/core/src/Transformation.js +++ b/packages/core/core/src/Transformation.js @@ -83,7 +83,8 @@ export type TransformationOpts = {| |}; export type TransformationResult = {| - assets: Array, + assets?: Array, + error?: Error, configRequests: Array, invalidations: Array, invalidateOnFileCreate: Array, @@ -178,19 +179,25 @@ export default class Transformation { asset.value.isSource, asset.value.pipeline, ); - let results = await this.runPipelines(pipeline, asset); - let assets = results.map(a => a.value); + let assets, error; + try { + let results = await this.runPipelines(pipeline, asset); + assets = results.map(a => a.value); + } catch (e) { + error = e; + } let configRequests = getConfigRequests([...this.configs.values()]); let devDepRequests = getWorkerDevDepRequests([ ...this.devDepRequests.values(), ]); - // $FlowFixMe + // $FlowFixMe because of $$raw return { $$raw: true, assets, configRequests, + error, invalidateOnFileCreate: this.invalidateOnFileCreate, invalidations: [...this.invalidations.values()], devDepRequests, diff --git a/packages/core/core/src/requests/AssetRequest.js b/packages/core/core/src/requests/AssetRequest.js index 07db6a1e69f..c378f907e59 100644 --- a/packages/core/core/src/requests/AssetRequest.js +++ b/packages/core/core/src/requests/AssetRequest.js @@ -128,6 +128,7 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) { let { assets, configRequests, + error, invalidations, invalidateOnFileCreate, devDepRequests, @@ -138,8 +139,10 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) { }): TransformationResult); let time = Date.now() - start; - for (let asset of assets) { - asset.stats.time = time; + if (assets) { + for (let asset of assets) { + asset.stats.time = time; + } } for (let invalidation of invalidateOnFileCreate) { @@ -171,5 +174,9 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) { await runConfigRequest(api, configRequest); } - return assets; + if (error != null) { + throw error; + } else { + return nullthrows(assets); + } } From d9a55b73ffccf7aa251d1db74a94f3a76654438c Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Wed, 12 Jan 2022 23:17:59 +0100 Subject: [PATCH 3/4] Correctly serialize ThrowableDiagnostic from transformers --- packages/core/core/src/Transformation.js | 7 +++++-- packages/core/core/src/requests/AssetRequest.js | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/core/src/Transformation.js b/packages/core/core/src/Transformation.js index 33bdfe0ebda..d8e90198d8f 100644 --- a/packages/core/core/src/Transformation.js +++ b/packages/core/core/src/Transformation.js @@ -26,9 +26,11 @@ import {Readable} from 'stream'; import nullthrows from 'nullthrows'; import logger, {PluginLogger} from '@parcel/logger'; import ThrowableDiagnostic, { + anyToDiagnostic, errorToDiagnostic, escapeMarkdown, md, + type Diagnostic, } from '@parcel/diagnostic'; import {SOURCEMAP_EXTENSIONS} from '@parcel/utils'; import {hashString} from '@parcel/hash'; @@ -84,7 +86,7 @@ export type TransformationOpts = {| export type TransformationResult = {| assets?: Array, - error?: Error, + error?: Array, configRequests: Array, invalidations: Array, invalidateOnFileCreate: Array, @@ -197,7 +199,8 @@ export default class Transformation { $$raw: true, assets, configRequests, - error, + // When throwing an error, this (de)serialization is done automatically by the WorkerFarm + error: error ? anyToDiagnostic(error) : undefined, invalidateOnFileCreate: this.invalidateOnFileCreate, invalidations: [...this.invalidations.values()], devDepRequests, diff --git a/packages/core/core/src/requests/AssetRequest.js b/packages/core/core/src/requests/AssetRequest.js index c378f907e59..a02541d092e 100644 --- a/packages/core/core/src/requests/AssetRequest.js +++ b/packages/core/core/src/requests/AssetRequest.js @@ -13,6 +13,7 @@ import type {ConfigAndCachePath} from './ParcelConfigRequest'; import type {TransformationResult} from '../Transformation'; import nullthrows from 'nullthrows'; +import ThrowableDiagnostic from '@parcel/diagnostic'; import {hashString} from '@parcel/hash'; import createParcelConfigRequest from './ParcelConfigRequest'; import {runDevDepRequest} from './DevDepRequest'; @@ -175,7 +176,7 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) { } if (error != null) { - throw error; + throw new ThrowableDiagnostic({diagnostic: error}); } else { return nullthrows(assets); } From df5852121d0cd107e436a8710d88e9164f8bb18a Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Sun, 16 Jan 2022 23:37:40 +0100 Subject: [PATCH 4/4] Cleanup --- packages/core/integration-tests/test/cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/integration-tests/test/cache.js b/packages/core/integration-tests/test/cache.js index 806251e35fe..667f00566bf 100644 --- a/packages/core/integration-tests/test/cache.js +++ b/packages/core/integration-tests/test/cache.js @@ -5862,7 +5862,7 @@ describe('cache', function () { invariant(event.type === 'buildFailure'); assert.strictEqual(event.diagnostics[0].message, 'Custom error'); - // Change included file back + // Clear transformer error await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'b'); event = await getNextBuild(b); invariant(event.type === 'buildSuccess');