Skip to content

Commit 5837373

Browse files
authoredJun 2, 2023
chore(cli): print detected cycle if work graph stalls (#25830)
To help with diagnosing #25806, if the work graph can't make any progress anymore because of a dependency cycle, print the cycle that was found instead of all the remaining nodes. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 2bcd7f2 commit 5837373

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed
 

‎packages/aws-cdk/lib/util/work-graph.ts

+38-2
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ export class WorkGraph {
272272
this.readyPool.sort((a, b) => (b.priority ?? 0) - (a.priority ?? 0));
273273

274274
if (this.readyPool.length === 0 && activeCount === 0 && pendingCount > 0) {
275-
throw new Error(`Unable to make progress anymore among: ${this}`);
275+
throw new Error(`Unable to make progress anymore, dependency cycle between remaining artifacts: ${this.findCycle().join(' -> ')}`);
276276
}
277277
}
278278

@@ -283,6 +283,42 @@ export class WorkGraph {
283283
}
284284
}
285285
}
286+
287+
/**
288+
* Find cycles in a graph
289+
*
290+
* Not the fastest, but effective and should be rare
291+
*/
292+
private findCycle(): string[] {
293+
const seen = new Set<string>();
294+
const self = this;
295+
for (const nodeId of Object.keys(this.nodes)) {
296+
const cycle = recurse(nodeId, [nodeId]);
297+
if (cycle) { return cycle; }
298+
}
299+
return ['No cycle found!'];
300+
301+
function recurse(nodeId: string, path: string[]): string[] | undefined {
302+
if (seen.has(nodeId)) {
303+
return undefined;
304+
}
305+
try {
306+
for (const dep of self.nodes[nodeId].dependencies ?? []) {
307+
const index = path.indexOf(dep);
308+
if (index > -1) {
309+
return [...path.slice(index), dep];
310+
}
311+
312+
const cycle = recurse(dep, [...path, dep]);
313+
if (cycle) { return cycle; }
314+
}
315+
316+
return undefined;
317+
} finally {
318+
seen.add(nodeId);
319+
}
320+
}
321+
}
286322
}
287323

288324
export interface WorkGraphActions {
@@ -297,4 +333,4 @@ function sum(xs: number[]) {
297333
ret += x;
298334
}
299335
return ret;
300-
}
336+
}

‎packages/aws-cdk/test/work-graph.test.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -328,13 +328,15 @@ describe('WorkGraph', () => {
328328
toDeploy: createArtifacts([
329329
{ id: 'A', type: 'stack', stackDependencies: ['A'] },
330330
]),
331+
expectedError: 'A -> A',
331332
},
332333
{
333334
scenario: 'A -> B, B -> A',
334335
toDeploy: createArtifacts([
335336
{ id: 'A', type: 'stack', stackDependencies: ['B'] },
336337
{ id: 'B', type: 'stack', stackDependencies: ['A'] },
337338
]),
339+
expectedError: 'A -> B -> A',
338340
},
339341
{
340342
scenario: 'A, B -> C, C -> D, D -> B',
@@ -344,12 +346,13 @@ describe('WorkGraph', () => {
344346
{ id: 'C', type: 'stack', stackDependencies: ['D'] },
345347
{ id: 'D', type: 'stack', stackDependencies: ['B'] },
346348
]),
349+
expectedError: 'B -> C -> D -> B',
347350
},
348-
])('Failure - Graph Circular Dependencies - $scenario', async ({ toDeploy }) => {
351+
])('Failure - Graph Circular Dependencies - $scenario', async ({ toDeploy, expectedError }) => {
349352
const graph = new WorkGraph();
350353
addTestArtifactsToGraph(toDeploy, graph);
351354

352-
await expect(graph.doParallel(1, callbacks)).rejects.toThrowError(/Unable to make progress anymore/);
355+
await expect(graph.doParallel(1, callbacks)).rejects.toThrowError(new RegExp(`Unable to make progress.*${expectedError}`));
353356
});
354357
});
355358

0 commit comments

Comments
 (0)
Please sign in to comment.