fix(core): improve sync error handling and messaging

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

Hide file tree

Showing 7 changed files with 318 additions and 148 deletions.
diff --git a/packages/nx/src/command-line/sync/sync.ts b/packages/nx/src/command-line/sync/sync.ts
@@ -7,6 +7,7 @@ import {
   collectAllRegisteredSyncGenerators,
   flushSyncGeneratorChanges,
   getFailedSyncGeneratorsFixMessageLines,
+  getFlushFailureMessageLines,
   getSyncGeneratorChanges,
   getSyncGeneratorSuccessResultsMessageLines,
   processSyncGeneratorResultErrors,
@@ -65,16 +66,16 @@ export function syncHandler(options: SyncOptions): Promise<number> {
     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 ${
+      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 0;
+      return 1;
     }
 
     const resultBodyLines = getSyncGeneratorSuccessResultsMessageLines(results);
@@ -85,7 +86,7 @@ export function syncHandler(options: SyncOptions): Promise<number> {
       });
 
       if (anySyncGeneratorsFailed) {
-        output.warn({
+        output.error({
           title:
             failedGeneratorsCount === 1
               ? 'A sync generator failed to run'
@@ -106,7 +107,17 @@ export function syncHandler(options: SyncOptions): Promise<number> {
     spinner.start();
 
     try {
-      await flushSyncGeneratorChanges(results);
+      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({
@@ -115,26 +126,36 @@ export function syncHandler(options: SyncOptions): Promise<number> {
           'Syncing the workspace failed with the following error:',
           '',
           e.message,
-          ...(options.verbose ? [`\n${e.stack}`] : []),
+          ...(options.verbose && !!e.stack ? [`\n${e.stack}`] : []),
+          '',
+          'Please make sure to run with `--verbose` and report the error at: https://github.com/nrwl/nx/issues/new/choose',
         ],
       });
 
       return 1;
     }
 
-    spinner.succeed(`The workspace was synced successfully!
-
-Please make sure to commit the changes to your repository.`);
+    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}`);
 
     if (anySyncGeneratorsFailed) {
-      output.warn({
-        title: `The workspace might still be out of sync because ${
+      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 { SyncGeneratorRunResult } from '../../utils/sync-generators';
+import type {
+  FlushSyncGeneratorChangesResult,
+  SyncGeneratorRunResult,
+} from '../../utils/sync-generators';
 import {
   GET_REGISTERED_SYNC_GENERATORS,
   type HandleGetRegisteredSyncGeneratorsMessage,
@@ -372,7 +375,9 @@ export class DaemonClient {
     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/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,6 +9,7 @@ import {
   collectRegisteredGlobalSyncGenerators,
   flushSyncGeneratorChanges,
   runSyncGenerator,
+  type FlushSyncGeneratorChangesResult,
   type SyncGeneratorRunResult,
   type SyncGeneratorRunSuccessResult,
 } from '../../utils/sync-generators';
@@ -71,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);
@@ -80,7 +81,7 @@ export async function flushSyncGeneratorChangesToDisk(
     syncGeneratorsCacheResultPromises.delete(generator);
   }
 
-  await flushSyncGeneratorChanges(results);
+  return await flushSyncGeneratorChanges(results);
 }
 
 export function collectAndScheduleSyncGenerators(