Skip to content

Commit f38b466

Browse files
Blaszlukebatchelor
authored andcommitted
Fix --no-bail not throwing when running in default orderMode (#256)
* Fix --no-bail not throwing when running in default orderMode * Try to fix flow errors on CI that I cannot reproduce locally * Extract error message into messages util * Fix flow errors
1 parent 84b636d commit f38b466

File tree

4 files changed

+101
-14
lines changed

4 files changed

+101
-14
lines changed

src/Project.js

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import * as logger from './utils/logger';
1010
import {
1111
promiseWrapper,
1212
promiseWrapperSuccess,
13-
type PromiseResult
13+
type PromiseResult,
14+
type PromiseFailure
1415
} from './utils/promiseWrapper';
1516
import * as messages from './utils/messages';
1617
import { BoltError } from './utils/errors';
@@ -22,7 +23,7 @@ import chunkd from 'chunkd';
2223

2324
type GenericTask<T> = (pkg: Package) => Promise<T>;
2425

25-
type TaskResult = PromiseResult<mixed>;
26+
type TaskResult = PromiseResult<mixed> | void;
2627

2728
type InternalTask = GenericTask<TaskResult>;
2829

@@ -197,12 +198,17 @@ export default class Project {
197198
spawnOpts.excludeFromGraph
198199
);
199200
}
200-
201-
results.forEach(r => {
202-
if (r.status === 'error') {
203-
throw r.error;
204-
}
205-
});
201+
const taskFailures: Array<PromiseFailure> = (results.filter(
202+
r => r && r.status === 'error'
203+
): any);
204+
if (taskFailures.length > 0) {
205+
const failuresWithMsg = taskFailures
206+
.map(r => r.error && r.error.message)
207+
.filter(Boolean);
208+
throw new BoltError(
209+
messages.taskFailed(taskFailures.length, failuresWithMsg)
210+
);
211+
}
206212
}
207213

208214
async runPackageTasksSerial<T>(
@@ -249,7 +255,7 @@ export default class Project {
249255
packages: Array<Package>,
250256
task: GenericTask<T>,
251257
excludeDepTypesFromGraph: Array<configDependencyType> | void
252-
): Promise<Array<T>> {
258+
): Promise<Array<T | void>> {
253259
let { graph: dependentsGraph, valid } = await this.getDependencyGraph(
254260
packages,
255261
excludeDepTypesFromGraph
@@ -261,7 +267,7 @@ export default class Project {
261267
graph.set(pkgName, pkgInfo.dependencies);
262268
}
263269

264-
let { safe, values } = await taskGraphRunner({
270+
let results = await taskGraphRunner({
265271
graph,
266272
force: true,
267273
task: async pkgName => {
@@ -272,10 +278,19 @@ export default class Project {
272278
}
273279
});
274280

281+
let {
282+
safe,
283+
values
284+
}: { safe: boolean, values: Map<string, T | void> } = (results: {
285+
safe: boolean,
286+
// The value type of the Map is wrong so we cast it to any and recast to the correct type
287+
values: Map<string, any>
288+
});
289+
275290
if (!safe) {
276291
logger.warn(messages.unsafeCycles());
277292
}
278-
return ((Object.values(values): any): Array<T>);
293+
return [...values.values()];
279294
}
280295

281296
getPackageByName(packages: Array<Package>, pkgName: string) {

src/__tests__/Project.test.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,71 @@ describe('Project', () => {
510510
done();
511511
});
512512

513+
test('runPackageTasks() fails with no bail orderMode: parallel', async done => {
514+
let project = await Project.init(f.find('independent-workspaces'));
515+
let packages = await project.getPackages();
516+
let ops = [];
517+
518+
try {
519+
await project.runPackageTasks(
520+
packages,
521+
{ orderMode: 'parallel', bail: false },
522+
async pkg => {
523+
ops.push('start:' + pkg.getName());
524+
if (pkg.getName() === 'bar') {
525+
throw new Error('test');
526+
}
527+
await Promise.resolve();
528+
ops.push('end:' + pkg.getName());
529+
}
530+
);
531+
done.fail(new Error('Error should have been thrown'));
532+
} catch (error) {
533+
expect(ops).toEqual(['start:bar', 'start:foo', 'end:foo']);
534+
}
535+
done();
536+
});
537+
538+
test('runPackageTasks() fails with no bail with no orderMode', async done => {
539+
let project = await Project.init(f.find('independent-workspaces'));
540+
let packages = await project.getPackages();
541+
let ops = [];
542+
543+
try {
544+
await project.runPackageTasks(packages, { bail: false }, async pkg => {
545+
ops.push('start:' + pkg.getName());
546+
if (pkg.getName() === 'bar') {
547+
throw new Error('test');
548+
}
549+
await Promise.resolve();
550+
ops.push('end:' + pkg.getName());
551+
});
552+
done.fail(new Error('Error should have been thrown'));
553+
} catch (error) {
554+
expect(ops).toEqual(['start:bar', 'start:foo', 'end:foo']);
555+
}
556+
done();
557+
});
558+
559+
test('runPackageTasks() reports decent error message when failing with no bail', async () => {
560+
let project = await Project.init(f.find('independent-workspaces'));
561+
let packages = await project.getPackages();
562+
let ops = [];
563+
564+
await expect(
565+
project.runPackageTasks(packages, { bail: false }, async pkg => {
566+
ops.push('start:' + pkg.getName());
567+
if (pkg.getName() === 'bar') {
568+
throw new Error('Bar is bad');
569+
}
570+
await Promise.resolve();
571+
ops.push('end:' + pkg.getName());
572+
})
573+
).rejects.toMatchObject({
574+
message: expect.stringMatching(/1 tasks failed.\nBar is bad/)
575+
});
576+
});
577+
513578
test('runPackageTasks() excludeFromGraph: devDependencies', async () => {
514579
let project = await Project.init(f.find('dev-dependent-workspaces-only'));
515580
let packages = await project.getPackages();

src/utils/messages.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,3 +408,10 @@ export function taskRunningAcrossCINodes(
408408
): Message {
409409
return `Task is being split across ${nodes} nodes. Current node running across ${count} of ${total} workspaces`;
410410
}
411+
412+
export function taskFailed(
413+
numFailures: number,
414+
failuresWithMsg: string[]
415+
): Message {
416+
return `${numFailures} tasks failed.\n${failuresWithMsg.join('\n')}`;
417+
}

src/utils/promiseWrapper.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// @flow
22

3-
type PromiseSuccess<T> = {
3+
export type PromiseSuccess<T> = {
44
result: T,
55
status: 'success'
66
};
77

8-
type PromiseFailure = {
9-
error: mixed,
8+
export type PromiseFailure = {
9+
error: any,
1010
status: 'error'
1111
};
1212

0 commit comments

Comments
 (0)