fix(core): handle sync generator failures

nrwl · Sep 5, 2024 · 3cf39e9 · 3cf39e9
1 parent ccda7f9
commit 3cf39e9
Show file tree

Hide file tree

Showing 6 changed files with 327 additions and 81 deletions.
diff --git a/packages/nx/src/command-line/sync/sync.ts b/packages/nx/src/command-line/sync/sync.ts
@@ -6,8 +6,10 @@ import { handleErrors } from '../../utils/params';
 import {
   collectAllRegisteredSyncGenerators,
   flushSyncGeneratorChanges,
+  getFailedSyncGeneratorsFixMessageLines,
   getSyncGeneratorChanges,
-  syncGeneratorResultsToMessageLines,
+  getSyncGeneratorSuccessResultsMessageLines,
+  processSyncGeneratorResultErrors,
 } from '../../utils/sync-generators';
 import type { SyncArgs } from './command-object';
 import chalk = require('chalk');
@@ -52,29 +54,88 @@ export function syncHandler(options: SyncOptions): Promise<number> {
       return 0;
     }
 
+    const {
+      failedGeneratorsCount,
+      areAllResultsFailures,
+      anySyncGeneratorsFailed,
+    } = processSyncGeneratorResultErrors(results);
+    const failedSyncGeneratorsFixMessageLines =
+      getFailedSyncGeneratorsFixMessageLines(results, options.verbose);
+
+    if (areAllResultsFailures) {
+      // if all sync generators failed to run we can't say for sure if the workspace is out of sync
+      // because they could have failed due to a bug, so we print a warning and exit with code 0
+      output.warn({
+        title: `The workspace might be out of sync because ${
+          failedGeneratorsCount === 1
+            ? 'a sync generator'
+            : 'some sync generators'
+        } failed to run`,
+        bodyLines: failedSyncGeneratorsFixMessageLines,
+      });
+
+      return 0;
+    }
+
+    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.warn({
+          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 {
+      await flushSyncGeneratorChanges(results);
+    } 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 ? [`\n${e.stack}`] : []),
+        ],
+      });
+
+      return 1;
+    }
 
     spinner.succeed(`The workspace was synced successfully!
 
-Please make sure to commit the changes to your repository.
-`);
+Please make sure to commit the changes to your repository.`);
+
+    if (anySyncGeneratorsFailed) {
+      output.warn({
+        title: `The workspace might still be out of sync because ${
+          failedGeneratorsCount === 1
+            ? 'a sync generator'
+            : 'some sync generators'
+        } failed to run`,
+        bodyLines: failedSyncGeneratorsFixMessageLines,
+      });
+    }
 
     return 0;
   });

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

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.ts b/packages/nx/src/daemon/server/sync-generators.ts
@@ -9,15 +9,16 @@ import {
   collectRegisteredGlobalSyncGenerators,
   flushSyncGeneratorChanges,
   runSyncGenerator,
-  type SyncGeneratorChangesResult,
+  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 +37,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
@@ -154,7 +155,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 +224,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 +247,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 +258,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 +321,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 +425,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 +436,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;
   });
 }