Skip to content

Commit 6094369

Browse files
authoredJan 17, 2022
Don't discard invalidations when transformer throws an error (#7547)
1 parent 0cf11f7 commit 6094369

File tree

4 files changed

+71
-7
lines changed

4 files changed

+71
-7
lines changed
 

‎packages/core/core/src/Transformation.js

+14-4
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ import {Readable} from 'stream';
2626
import nullthrows from 'nullthrows';
2727
import logger, {PluginLogger} from '@parcel/logger';
2828
import ThrowableDiagnostic, {
29+
anyToDiagnostic,
2930
errorToDiagnostic,
3031
escapeMarkdown,
3132
md,
33+
type Diagnostic,
3234
} from '@parcel/diagnostic';
3335
import {SOURCEMAP_EXTENSIONS} from '@parcel/utils';
3436
import {hashString} from '@parcel/hash';
@@ -83,7 +85,8 @@ export type TransformationOpts = {|
8385
|};
8486

8587
export type TransformationResult = {|
86-
assets: Array<AssetValue>,
88+
assets?: Array<AssetValue>,
89+
error?: Array<Diagnostic>,
8790
configRequests: Array<ConfigRequest>,
8891
invalidations: Array<RequestInvalidation>,
8992
invalidateOnFileCreate: Array<InternalFileCreateInvalidation>,
@@ -178,19 +181,26 @@ export default class Transformation {
178181
asset.value.isSource,
179182
asset.value.pipeline,
180183
);
181-
let results = await this.runPipelines(pipeline, asset);
182-
let assets = results.map(a => a.value);
184+
let assets, error;
185+
try {
186+
let results = await this.runPipelines(pipeline, asset);
187+
assets = results.map(a => a.value);
188+
} catch (e) {
189+
error = e;
190+
}
183191

184192
let configRequests = getConfigRequests([...this.configs.values()]);
185193
let devDepRequests = getWorkerDevDepRequests([
186194
...this.devDepRequests.values(),
187195
]);
188196

189-
// $FlowFixMe
197+
// $FlowFixMe because of $$raw
190198
return {
191199
$$raw: true,
192200
assets,
193201
configRequests,
202+
// When throwing an error, this (de)serialization is done automatically by the WorkerFarm
203+
error: error ? anyToDiagnostic(error) : undefined,
194204
invalidateOnFileCreate: this.invalidateOnFileCreate,
195205
invalidations: [...this.invalidations.values()],
196206
devDepRequests,

‎packages/core/core/src/requests/AssetRequest.js

+11-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type {ConfigAndCachePath} from './ParcelConfigRequest';
1313
import type {TransformationResult} from '../Transformation';
1414

1515
import nullthrows from 'nullthrows';
16+
import ThrowableDiagnostic from '@parcel/diagnostic';
1617
import {hashString} from '@parcel/hash';
1718
import createParcelConfigRequest from './ParcelConfigRequest';
1819
import {runDevDepRequest} from './DevDepRequest';
@@ -128,6 +129,7 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) {
128129
let {
129130
assets,
130131
configRequests,
132+
error,
131133
invalidations,
132134
invalidateOnFileCreate,
133135
devDepRequests,
@@ -138,8 +140,10 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) {
138140
}): TransformationResult);
139141

140142
let time = Date.now() - start;
141-
for (let asset of assets) {
142-
asset.stats.time = time;
143+
if (assets) {
144+
for (let asset of assets) {
145+
asset.stats.time = time;
146+
}
143147
}
144148

145149
for (let invalidation of invalidateOnFileCreate) {
@@ -171,5 +175,9 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) {
171175
await runConfigRequest(api, configRequest);
172176
}
173177

174-
return assets;
178+
if (error != null) {
179+
throw new ThrowableDiagnostic({diagnostic: error});
180+
} else {
181+
return nullthrows(assets);
182+
}
175183
}

‎packages/core/integration-tests/test/cache.js

+43
Original file line numberDiff line numberDiff line change
@@ -5836,6 +5836,49 @@ describe('cache', function () {
58365836
}
58375837
});
58385838

5839+
it('properly watches included files after a transformer error', async function () {
5840+
this.timeout(15000);
5841+
let subscription;
5842+
let fixture = path.join(__dirname, '/integration/included-file');
5843+
try {
5844+
let b = bundler(path.join(fixture, 'index.txt'), {
5845+
inputFS: overlayFS,
5846+
shouldDisableCache: false,
5847+
});
5848+
await overlayFS.mkdirp(fixture);
5849+
await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'a');
5850+
subscription = await b.watch();
5851+
let event = await getNextBuild(b);
5852+
invariant(event.type === 'buildSuccess');
5853+
let output1 = await overlayFS.readFile(
5854+
event.bundleGraph.getBundles()[0].filePath,
5855+
'utf8',
5856+
);
5857+
assert.strictEqual(output1, 'a');
5858+
5859+
// Change included file
5860+
await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'ERROR');
5861+
event = await getNextBuild(b);
5862+
invariant(event.type === 'buildFailure');
5863+
assert.strictEqual(event.diagnostics[0].message, 'Custom error');
5864+
5865+
// Clear transformer error
5866+
await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'b');
5867+
event = await getNextBuild(b);
5868+
invariant(event.type === 'buildSuccess');
5869+
let output3 = await overlayFS.readFile(
5870+
event.bundleGraph.getBundles()[0].filePath,
5871+
'utf8',
5872+
);
5873+
assert.strictEqual(output3, 'b');
5874+
} finally {
5875+
if (subscription) {
5876+
await subscription.unsubscribe();
5877+
subscription = null;
5878+
}
5879+
}
5880+
});
5881+
58395882
it('should support moving the project root', async function () {
58405883
// This test relies on the real filesystem because the memory fs doesn't support renames.
58415884
// But renameSync is broken on windows in CI with EPERM errors. Just skip this test for now.

‎packages/core/integration-tests/test/integration/included-file/node_modules/parcel-transformer-included/index.js

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
Please sign in to comment.