fix(core): handle sync generator failures (nrwl#27650)

XavierMP14 · Sep 10, 2024 · 4986b88 · 4986b88
1 parent 5d039c2
commit 4986b88
Show file tree

Hide file tree

Showing 8 changed files with 558 additions and 144 deletions.
diff --git a/packages/nx/src/command-line/sync/sync.ts b/packages/nx/src/command-line/sync/sync.ts
@@ -6,8 +6,11 @@ import { handleErrors } from '../../utils/params';
 import {
   collectAllRegisteredSyncGenerators,
   flushSyncGeneratorChanges,
+  getFailedSyncGeneratorsFixMessageLines,
+  getFlushFailureMessageLines,
   getSyncGeneratorChanges,
-  syncGeneratorResultsToMessageLines,
+  getSyncGeneratorSuccessResultsMessageLines,
+  processSyncGeneratorResultErrors,
 } from '../../utils/sync-generators';
 import type { SyncArgs } from './command-object';
 import chalk = require('chalk');
@@ -52,29 +55,106 @@ export function syncHandler(options: SyncOptions): Promise<number> {
       return 0;
     }
 
+    const {
+      failedGeneratorsCount,
+      areAllResultsFailures,
+      anySyncGeneratorsFailed,
+    } = processSyncGeneratorResultErrors(results);
+    const failedSyncGeneratorsFixMessageLines =
+      getFailedSyncGeneratorsFixMessageLines(results, options.verbose);
+
+    if (areAllResultsFailures) {
+      output.error({
+        title: `The workspace is probably out of sync because ${
+          failedGeneratorsCount === 1
+            ? 'a sync generator'
+            : 'some sync generators'
+        } failed to run`,
+        bodyLines: failedSyncGeneratorsFixMessageLines,
+      });
+
+      return 1;
+    }
+
+    const resultBodyLines = getSyncGeneratorSuccessResultsMessageLines(results);
     if (options.check) {
       output.error({
-        title: `The workspace is out of sync`,
-        bodyLines: syncGeneratorResultsToMessageLines(results),
+        title: 'The workspace is out of sync',
+        bodyLines: resultBodyLines,
       });
 
+      if (anySyncGeneratorsFailed) {
+        output.error({
+          title:
+            failedGeneratorsCount === 1
+              ? 'A sync generator failed to run'
+              : 'Some sync generators failed to run',
+          bodyLines: failedSyncGeneratorsFixMessageLines,
+        });
+      }
+
       return 1;
     }
 
     output.warn({
-      title: `The workspace is out of sync`,
-      bodyLines: syncGeneratorResultsToMessageLines(results),
+      title: 'The workspace is out of sync',
+      bodyLines: resultBodyLines,
     });
 
     const spinner = ora('Syncing the workspace...');
     spinner.start();
 
-    await flushSyncGeneratorChanges(results);
+    try {
+      const flushResult = await flushSyncGeneratorChanges(results);
+
+      if ('generatorFailures' in flushResult) {
+        spinner.fail();
+        output.error({
+          title: 'Failed to sync the workspace',
+          bodyLines: getFlushFailureMessageLines(flushResult, options.verbose),
+        });
+
+        return 1;
+      }
+    } catch (e) {
+      spinner.fail();
+      output.error({
+        title: 'Failed to sync the workspace',
+        bodyLines: [
+          'Syncing the workspace failed with the following error:',
+          '',
+          e.message,
+          ...(options.verbose && !!e.stack ? [`\n${e.stack}`] : []),
+          '',
+          'Please rerun with `--verbose` and report the error at: https://github.com/nrwl/nx/issues/new/choose',
+        ],
+      });
+
+      return 1;
+    }
 
-    spinner.succeed(`The workspace was synced successfully!
+    const successTitle = anySyncGeneratorsFailed
+      ? // the identified changes were synced successfully, but the workspace
+        // is still not up to date, which we'll mention next
+        'The identified changes were synced successfully!'
+      : // the workspace is fully up to date
+        'The workspace was synced successfully!';
+    const successSubtitle =
+      'Please make sure to commit the changes to your repository.';
+    spinner.succeed(`${successTitle}\n\n${successSubtitle}`);
 
-Please make sure to commit the changes to your repository.
-`);
+    if (anySyncGeneratorsFailed) {
+      output.error({
+        title: `The workspace is probably still out of sync because ${
+          failedGeneratorsCount === 1
+            ? 'a sync generator'
+            : 'some sync generators'
+        } failed to run`,
+        bodyLines: failedSyncGeneratorsFixMessageLines,
+      });
+
+      return 1;
+    }
 
     return 0;
   });

diff --git a/packages/nx/src/daemon/client/client.ts b/packages/nx/src/daemon/client/client.ts
@@ -54,7 +54,10 @@ import {
   GET_SYNC_GENERATOR_CHANGES,
   type HandleGetSyncGeneratorChangesMessage,
 } from '../message-types/get-sync-generator-changes';
-import type { SyncGeneratorChangesResult } from '../../utils/sync-generators';
+import type {
+  FlushSyncGeneratorChangesResult,
+  SyncGeneratorRunResult,
+} from '../../utils/sync-generators';
 import {
   GET_REGISTERED_SYNC_GENERATORS,
   type HandleGetRegisteredSyncGeneratorsMessage,
@@ -364,15 +367,17 @@ export class DaemonClient {
 
   getSyncGeneratorChanges(
     generators: string[]
-  ): Promise<SyncGeneratorChangesResult[]> {
+  ): Promise<SyncGeneratorRunResult[]> {
     const message: HandleGetSyncGeneratorChangesMessage = {
       type: GET_SYNC_GENERATOR_CHANGES,
       generators,
     };
     return this.sendToDaemonViaQueue(message);
   }
 
-  flushSyncGeneratorChangesToDisk(generators: string[]): Promise<void> {
+  flushSyncGeneratorChangesToDisk(
+    generators: string[]
+  ): Promise<FlushSyncGeneratorChangesResult> {
     const message: HandleFlushSyncGeneratorChangesToDiskMessage = {
       type: FLUSH_SYNC_GENERATOR_CHANGES_TO_DISK,
       generators,

diff --git a/packages/nx/src/daemon/server/handle-flush-sync-generator-changes-to-disk.ts b/packages/nx/src/daemon/server/handle-flush-sync-generator-changes-to-disk.ts
@@ -4,10 +4,10 @@ import { flushSyncGeneratorChangesToDisk } from './sync-generators';
 export async function handleFlushSyncGeneratorChangesToDisk(
   generators: string[]
 ): Promise<HandlerResult> {
-  await flushSyncGeneratorChangesToDisk(generators);
+  const result = await flushSyncGeneratorChangesToDisk(generators);
 
   return {
-    response: '{}',
+    response: JSON.stringify(result),
     description: 'handleFlushSyncGeneratorChangesToDisk',
   };
 }
diff --git a/packages/nx/src/daemon/server/handle-get-sync-generator-changes.ts b/packages/nx/src/daemon/server/handle-get-sync-generator-changes.ts
@@ -6,12 +6,16 @@ export async function handleGetSyncGeneratorChanges(
 ): Promise<HandlerResult> {
   const changes = await getCachedSyncGeneratorChanges(generators);
 
-  // strip out the content of the changes and any potential callback
-  const result = changes.map((change) => ({
-    generatorName: change.generatorName,
-    changes: change.changes.map((c) => ({ ...c, content: null })),
-    outOfSyncMessage: change.outOfSyncMessage,
-  }));
+  const result = changes.map((change) =>
+    'error' in change
+      ? change
+      : // strip out the content of the changes and any potential callback
+        {
+          generatorName: change.generatorName,
+          changes: change.changes.map((c) => ({ ...c, content: null })),
+          outOfSyncMessage: change.outOfSyncMessage,
+        }
+  );
 
   return {
     response: JSON.stringify(result),

diff --git a/packages/nx/src/daemon/server/sync-generators.spec.ts b/packages/nx/src/daemon/server/sync-generators.spec.ts
@@ -1,9 +1,9 @@
-import type { SyncGeneratorChangesResult } from '../../utils/sync-generators';
+import type { SyncGeneratorRunResult } from '../../utils/sync-generators';
 import { _getConflictingGeneratorGroups } from './sync-generators';
 
 describe('_getConflictingGeneratorGroups', () => {
   it('should return grouped conflicting generators', () => {
-    const results: SyncGeneratorChangesResult[] = [
+    const results: SyncGeneratorRunResult[] = [
       {
         generatorName: 'a',
         changes: [

diff --git a/packages/nx/src/daemon/server/sync-generators.ts b/packages/nx/src/daemon/server/sync-generators.ts
@@ -9,15 +9,17 @@ import {
   collectRegisteredGlobalSyncGenerators,
   flushSyncGeneratorChanges,
   runSyncGenerator,
-  type SyncGeneratorChangesResult,
+  type FlushSyncGeneratorChangesResult,
+  type SyncGeneratorRunResult,
+  type SyncGeneratorRunSuccessResult,
 } from '../../utils/sync-generators';
 import { workspaceRoot } from '../../utils/workspace-root';
 import { serverLogger } from './logger';
 import { getCachedSerializedProjectGraphPromise } from './project-graph-incremental-recomputation';
 
 const syncGeneratorsCacheResultPromises = new Map<
   string,
-  Promise<SyncGeneratorChangesResult>
+  Promise<SyncGeneratorRunResult>
 >();
 let registeredTaskSyncGenerators = new Set<string>();
 let registeredGlobalSyncGenerators = new Set<string>();
@@ -36,7 +38,7 @@ const log = (...messageParts: unknown[]) => {
 
 export async function getCachedSyncGeneratorChanges(
   generators: string[]
-): Promise<SyncGeneratorChangesResult[]> {
+): Promise<SyncGeneratorRunResult[]> {
   try {
     log('get sync generators changes on demand', generators);
     // this is invoked imperatively, so we clear any scheduled run
@@ -70,7 +72,7 @@ export async function getCachedSyncGeneratorChanges(
 
 export async function flushSyncGeneratorChangesToDisk(
   generators: string[]
-): Promise<void> {
+): Promise<FlushSyncGeneratorChangesResult> {
   log('flush sync generators changes', generators);
 
   const results = await getCachedSyncGeneratorChanges(generators);
@@ -79,7 +81,7 @@ export async function flushSyncGeneratorChangesToDisk(
     syncGeneratorsCacheResultPromises.delete(generator);
   }
 
-  await flushSyncGeneratorChanges(results);
+  return await flushSyncGeneratorChanges(results);
 }
 
 export function collectAndScheduleSyncGenerators(
@@ -154,7 +156,7 @@ export async function getCachedRegisteredSyncGenerators(): Promise<string[]> {
 
 async function getFromCacheOrRunGenerators(
   generators: string[]
-): Promise<SyncGeneratorChangesResult[]> {
+): Promise<SyncGeneratorRunResult[]> {
   let projects: Record<string, ProjectConfiguration> | null;
   let errored = false;
   const getProjectsConfigurations = async () => {
@@ -223,7 +225,7 @@ async function getFromCacheOrRunGenerators(
 async function runConflictingGenerators(
   tree: Tree,
   generators: string[]
-): Promise<SyncGeneratorChangesResult[]> {
+): Promise<SyncGeneratorRunResult[]> {
   const { projectGraph } = await getCachedSerializedProjectGraphPromise();
   const projects = projectGraph
     ? readProjectsConfigurationFromProjectGraph(projectGraph).projects
@@ -246,7 +248,7 @@ async function runConflictingGenerators(
   }
 
   // we need to run conflicting generators sequentially because they use the same tree
-  const results: SyncGeneratorChangesResult[] = [];
+  const results: SyncGeneratorRunResult[] = [];
   for (const generator of generators) {
     log(generator, 'running it now');
     results.push(await runGenerator(generator, projects, tree));
@@ -257,16 +259,17 @@ async function runConflictingGenerators(
 
 async function processConflictingGenerators(
   conflicts: string[][],
-  initialResults: SyncGeneratorChangesResult[]
-): Promise<SyncGeneratorChangesResult[]> {
+  initialResults: SyncGeneratorRunResult[]
+): Promise<SyncGeneratorRunResult[]> {
   const conflictRunResults = (
     await Promise.all(
       conflicts.map((generators) => {
         const [firstGenerator, ...generatorsToRun] = generators;
         // it must exists because the conflicts were identified from the initial results
+        // and it's guaranteed to be a success result
         const firstGeneratorResult = initialResults.find(
           (r) => r.generatorName === firstGenerator
-        )!;
+        )! as SyncGeneratorRunSuccessResult;
 
         const tree = new FsTree(
           workspaceRoot,
@@ -319,10 +322,14 @@ async function processConflictingGenerators(
  * @internal
  */
 export function _getConflictingGeneratorGroups(
-  results: SyncGeneratorChangesResult[]
+  results: SyncGeneratorRunResult[]
 ): string[][] {
   const changedFileToGeneratorMap = new Map<string, Set<string>>();
   for (const result of results) {
+    if ('error' in result) {
+      continue;
+    }
+
     for (const change of result.changes) {
       if (!changedFileToGeneratorMap.has(change.path)) {
         changedFileToGeneratorMap.set(change.path, new Set());
@@ -419,7 +426,7 @@ function runGenerator(
   generator: string,
   projects: Record<string, ProjectConfiguration>,
   tree?: Tree
-): Promise<SyncGeneratorChangesResult> {
+): Promise<SyncGeneratorRunResult> {
   log('running scheduled generator', generator);
   // remove it from the scheduled set
   scheduledGenerators.delete(generator);
@@ -430,7 +437,11 @@ function runGenerator(
   );
 
   return runSyncGenerator(tree, generator, projects).then((result) => {
-    log(generator, 'changes:', result.changes.map((c) => c.path).join(', '));
+    if ('error' in result) {
+      log(generator, 'error:', result.error.message);
+    } else {
+      log(generator, 'changes:', result.changes.map((c) => c.path).join(', '));
+    }
     return result;
   });
 }