fix(ngcc): do not crash on entry-point that fails to compile (#36083)

Previously, when an entry-point contained code that caused its compilation
to fail, ngcc would exit in the middle of processing, possibly leaving other
entry-points in a corrupt state.

This change adds a new `errorOnFailedEntryPoint` option to `mainNgcc` that
specifies whether ngcc should exit immediately or log an error and continue
processing other entry-points.

The default is `false` so that ngcc will not error but continue processing
as much as possible. This is useful in post-install hooks, and async CLI
integration, where we do not have as much control over which entry-points
should be processed.

The option is forced to true if the `targetEntryPointPath` is provided,
such as the sync integration with the CLI, since in that case it is targeting
an entry-point that will actually be used in the current project so we do want
ngcc to exit with an error at that point.

PR Close #36083

packages/compiler-cli/ngcc/main-ngcc.ts

@@ -39,7 +39,8 @@ if (require.main === module) {
           .option('t', {
             alias: 'target',
             describe:
-                'A relative path (from the `source` path) to a single entry-point to process (plus its dependencies).',
+                'A relative path (from the `source` path) to a single entry-point to process (plus its dependencies).\n' +
+                'If this property is provided then `error-on-failed-entry-point` is forced to true',
           })
           .option('first-only', {
             describe:
@@ -83,6 +84,14 @@ if (require.main === module) {
             type: 'boolean',
             default: false,
           })
+          .option('error-on-failed-entry-point', {
+            describe:
+                'Set this option in order to terminate immediately with an error code if an entry-point fails to be processed.\n' +
+                'If `-t`/`--target` is provided then this property is always true and cannot be changed. Otherwise the default is false.\n' +
+                'When set to false, ngcc will continue to process entry-points after a failure. In which case it will log an error and resume processing other entry-points.',
+            type: 'boolean',
+            default: false,
+          })
           .strict()
           .help()
           .parse(args);
@@ -103,6 +112,7 @@ if (require.main === module) {
   const logLevel = options['l'] as keyof typeof LogLevel | undefined;
   const enableI18nLegacyMessageIdFormat = options['legacy-message-ids'];
   const invalidateEntryPointManifest = options['invalidate-entry-point-manifest'];
+  const errorOnFailedEntryPoint = options['error-on-failed-entry-point'];
 
   (async() => {
     try {
@@ -116,7 +126,7 @@ if (require.main === module) {
         createNewEntryPointFormats,
         logger,
         enableI18nLegacyMessageIdFormat,
-        async: options['async'], invalidateEntryPointManifest,
+        async: options['async'], invalidateEntryPointManifest, errorOnFailedEntryPoint,
       });
 
       if (logger) {

packages/compiler-cli/ngcc/src/execution/tasks/api.ts

@@ -110,6 +110,13 @@ export interface TaskQueue {
    */
   markTaskCompleted(task: Task): void;
 
+  /**
+   * Mark a task as failed.
+   *
+   * Do not process the tasks that depend upon the given task.
+   */
+  markAsFailed(task: Task): void;
+
   /**
    * Return a string representation of the task queue (for debugging purposes).
    *

packages/compiler-cli/ngcc/src/execution/tasks/completion.ts

@@ -6,10 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {FileSystem, resolve} from '../../../../src/ngtsc/file_system';
+import {Logger} from '../../logging/logger';
 import {markAsProcessed} from '../../packages/build_marker';
 import {PackageJsonFormatProperties, getEntryPointFormat} from '../../packages/entry_point';
 import {PackageJsonUpdater} from '../../writing/package_json_updater';
-import {Task, TaskCompletedCallback, TaskProcessingOutcome} from './api';
+import {Task, TaskCompletedCallback, TaskProcessingOutcome, TaskQueue} from './api';
 
 /**
  * A function that can handle a specific outcome of a task completion.
@@ -70,3 +71,17 @@ export function createThrowErrorHandler(fs: FileSystem): TaskCompletedHandler {
         (message !== null ? ` due to ${message}` : ''));
   };
 }
+
+/**
+ * Create a handler that logs an error and marks the task as failed.
+ */
+export function createLogErrorHandler(
+    logger: Logger, fs: FileSystem, taskQueue: TaskQueue): TaskCompletedHandler {
+  return (task: Task, message: string | null): void => {
+    taskQueue.markAsFailed(task);
+    const format = getEntryPointFormat(fs, task.entryPoint, task.formatProperty);
+    logger.error(
+        `Failed to compile entry-point ${task.entryPoint.name} (${task.formatProperty} as ${format})` +
+        (message !== null ? ` due to ${message}` : ''));
+  };
+}

packages/compiler-cli/ngcc/src/execution/tasks/queues/base_task_queue.ts

@@ -5,7 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-
+import {Logger} from '../../../logging/logger';
 import {PartiallyOrderedTasks, Task, TaskDependencies, TaskQueue} from '../api';
 import {stringifyTask} from '../utils';
 
@@ -19,9 +19,40 @@ export abstract class BaseTaskQueue implements TaskQueue {
   }
   protected inProgressTasks = new Set<Task>();
 
-  constructor(protected tasks: PartiallyOrderedTasks, protected dependencies: TaskDependencies) {}
+  /**
+   * A map of tasks that should be skipped, mapped to the task that caused them to be skipped.
+   */
+  private tasksToSkip = new Map<Task, Task>();
 
-  abstract getNextTask(): Task|null;
+  constructor(
+      protected logger: Logger, protected tasks: PartiallyOrderedTasks,
+      protected dependencies: TaskDependencies) {}
+
+  protected abstract computeNextTask(): Task|null;
+
+  getNextTask(): Task|null {
+    let nextTask = this.computeNextTask();
+    while (nextTask !== null) {
+      if (!this.tasksToSkip.has(nextTask)) {
+        break;
+      }
+      // We are skipping this task so mark it as complete
+      this.markTaskCompleted(nextTask);
+      const failedTask = this.tasksToSkip.get(nextTask) !;
+      this.logger.warn(
+          `Skipping processing of ${nextTask.entryPoint.name} because its dependency ${failedTask.entryPoint.name} failed to compile.`);
+      nextTask = this.computeNextTask();
+    }
+    return nextTask;
+  }
+
+  markAsFailed(task: Task) {
+    if (this.dependencies.has(task)) {
+      for (const dependentTask of this.dependencies.get(task) !) {
+        this.skipDependentTasks(dependentTask, task);
+      }
+    }
+  }
 
   markTaskCompleted(task: Task): void {
     if (!this.inProgressTasks.has(task)) {
@@ -41,6 +72,21 @@ export abstract class BaseTaskQueue implements TaskQueue {
         `  In-progress tasks (${inProgTasks.length}): ${this.stringifyTasks(inProgTasks, '    ')}`;
   }
 
+  /**
+   * Mark the given `task` as to be skipped, then recursive skip all its dependents.
+   *
+   * @param task The task to skip
+   * @param failedTask The task that failed, causing this task to be skipped
+   */
+  protected skipDependentTasks(task: Task, failedTask: Task) {
+    this.tasksToSkip.set(task, failedTask);
+    if (this.dependencies.has(task)) {
+      for (const dependentTask of this.dependencies.get(task) !) {
+        this.skipDependentTasks(dependentTask, failedTask);
+      }
+    }
+  }
+
   protected stringifyTasks(tasks: Task[], indentation: string): string {
     return tasks.map(task => `\n${indentation}- ${stringifyTask(task)}`).join('');
   }

packages/compiler-cli/ngcc/src/execution/tasks/queues/parallel_task_queue.ts

@@ -5,6 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
+import {Logger} from '../../../logging/logger';
 import {PartiallyOrderedTasks, Task, TaskDependencies} from '../api';
 import {getBlockedTasks, sortTasksByPriority, stringifyTask} from '../utils';
 import {BaseTaskQueue} from './base_task_queue';
@@ -21,12 +22,12 @@ export class ParallelTaskQueue extends BaseTaskQueue {
    */
   private blockedTasks: Map<Task, Set<Task>>;
 
-  constructor(tasks: PartiallyOrderedTasks, dependents: TaskDependencies) {
+  constructor(logger: Logger, tasks: PartiallyOrderedTasks, dependencies: TaskDependencies) {
-    super(sortTasksByPriority(tasks, dependents), dependents);
+    super(logger, sortTasksByPriority(tasks, dependencies), dependencies);
-    this.blockedTasks = getBlockedTasks(dependents);
+    this.blockedTasks = getBlockedTasks(dependencies);
   }
 
-  getNextTask(): Task|null {
+  computeNextTask(): Task|null {
     // Look for the first available (i.e. not blocked) task.
     // (NOTE: Since tasks are sorted by priority, the first available one is the best choice.)
     const nextTaskIdx = this.tasks.findIndex(task => !this.blockedTasks.has(task));

packages/compiler-cli/ngcc/src/execution/tasks/queues/serial_task_queue.ts

@@ -17,7 +17,7 @@ import {BaseTaskQueue} from './base_task_queue';
  * before requesting the next one.
  */
 export class SerialTaskQueue extends BaseTaskQueue {
-  getNextTask(): Task|null {
+  computeNextTask(): Task|null {
     const nextTask = this.tasks.shift() || null;
 
     if (nextTask) {

packages/compiler-cli/ngcc/src/main.ts

@@ -29,7 +29,7 @@ import {ClusterExecutor} from './execution/cluster/executor';
 import {ClusterPackageJsonUpdater} from './execution/cluster/package_json_updater';
 import {SingleProcessExecutorAsync, SingleProcessExecutorSync} from './execution/single_process_executor';
 import {CreateTaskCompletedCallback, PartiallyOrderedTasks, Task, TaskProcessingOutcome, TaskQueue} from './execution/tasks/api';
-import {composeTaskCompletedCallbacks, createMarkAsProcessedHandler, createThrowErrorHandler} from './execution/tasks/completion';
+import {composeTaskCompletedCallbacks, createLogErrorHandler, createMarkAsProcessedHandler, createThrowErrorHandler} from './execution/tasks/completion';
 import {ParallelTaskQueue} from './execution/tasks/queues/parallel_task_queue';
 import {SerialTaskQueue} from './execution/tasks/queues/serial_task_queue';
 import {computeTaskDependencies} from './execution/tasks/utils';
@@ -63,6 +63,8 @@ export interface SyncNgccOptions {
    * `basePath`.
    *
    * All its dependencies will need to be processed too.
+   *
+   * If this property is provided then `errorOnFailedEntryPoint` is forced to true.
    */
   targetEntryPointPath?: string;
 
@@ -108,6 +110,18 @@ export interface SyncNgccOptions {
    */
   async?: false;
 
+  /**
+   * Set to true in order to terminate immediately with an error code if an entry-point fails to be
+   * processed.
+   *
+   * If `targetEntryPointPath` is provided then this property is always true and cannot be
+   * changed. Otherwise the default is false.
+   *
+   * When set to false, ngcc will continue to process entry-points after a failure. In which case it
+   * will log an error and resume processing other entry-points.
+   */
+  errorOnFailedEntryPoint?: boolean;
+
   /**
    * Render `$localize` messages with legacy format ids.
    *
@@ -155,8 +169,13 @@ export function mainNgcc({basePath, targetEntryPointPath,
                           propertiesToConsider = SUPPORTED_FORMAT_PROPERTIES,
                           compileAllFormats = true, createNewEntryPointFormats = false,
                           logger = new ConsoleLogger(LogLevel.info), pathMappings, async = false,
-                          enableI18nLegacyMessageIdFormat = true,
+                          errorOnFailedEntryPoint = false, enableI18nLegacyMessageIdFormat = true,
                           invalidateEntryPointManifest = false}: NgccOptions): void|Promise<void> {
+  if (!!targetEntryPointPath) {
+    // targetEntryPointPath forces us to error if an entry-point fails.
+    errorOnFailedEntryPoint = true;
+  }
+
   // Execute in parallel, if async execution is acceptable and there are more than 1 CPU cores.
   const inParallel = async && (os.cpus().length > 1);
 
@@ -253,7 +272,7 @@ export function mainNgcc({basePath, targetEntryPointPath,
         `Analyzed ${entryPoints.length} entry-points in ${duration}s. ` +
         `(Total tasks: ${tasks.length})`);
 
-    return getTaskQueue(inParallel, tasks, graph);
+    return getTaskQueue(logger, inParallel, tasks, graph);
   };
 
   // The function for creating the `compile()` function.
@@ -294,20 +313,21 @@ export function mainNgcc({basePath, targetEntryPointPath,
               ts.formatDiagnosticsWithColorAndContext(result.diagnostics, bundle.src.host)));
         }
         fileWriter.writeBundle(bundle, result.transformedFiles, formatPropertiesToMarkAsProcessed);
+
+        logger.debug(`  Successfully compiled ${entryPoint.name} : ${formatProperty}`);
+
+        onTaskCompleted(task, TaskProcessingOutcome.Processed, null);
       } else {
         const errors = replaceTsWithNgInErrors(
             ts.formatDiagnosticsWithColorAndContext(result.diagnostics, bundle.src.host));
         onTaskCompleted(task, TaskProcessingOutcome.Failed, `compilation errors:\n${errors}`);
       }
-
-      logger.debug(`  Successfully compiled ${entryPoint.name} : ${formatProperty}`);
-
-      onTaskCompleted(task, TaskProcessingOutcome.Processed, null);
     };
   };
 
   // The executor for actually planning and getting the work done.
-  const createTaskCompletedCallback = getCreateTaskCompletedCallback(pkgJsonUpdater, fileSystem);
+  const createTaskCompletedCallback =
+      getCreateTaskCompletedCallback(pkgJsonUpdater, errorOnFailedEntryPoint, logger, fileSystem);
   const executor = getExecutor(
       async, inParallel, logger, pkgJsonUpdater, fileSystem, createTaskCompletedCallback);
 
@@ -349,17 +369,21 @@ function getFileWriter(
 }
 
 function getTaskQueue(
-    inParallel: boolean, tasks: PartiallyOrderedTasks, graph: DepGraph<EntryPoint>): TaskQueue {
+    logger: Logger, inParallel: boolean, tasks: PartiallyOrderedTasks,
+    graph: DepGraph<EntryPoint>): TaskQueue {
   const dependencies = computeTaskDependencies(tasks, graph);
-  return inParallel ? new ParallelTaskQueue(tasks, dependencies) :
+  return inParallel ? new ParallelTaskQueue(logger, tasks, dependencies) :
-                      new SerialTaskQueue(tasks, dependencies);
+                      new SerialTaskQueue(logger, tasks, dependencies);
 }
 
 function getCreateTaskCompletedCallback(
-    pkgJsonUpdater: PackageJsonUpdater, fileSystem: FileSystem): CreateTaskCompletedCallback {
+    pkgJsonUpdater: PackageJsonUpdater, errorOnFailedEntryPoint: boolean, logger: Logger,
-  return _taskQueue => composeTaskCompletedCallbacks({
+    fileSystem: FileSystem): CreateTaskCompletedCallback {
+  return taskQueue => composeTaskCompletedCallbacks({
            [TaskProcessingOutcome.Processed]: createMarkAsProcessedHandler(pkgJsonUpdater),
-           [TaskProcessingOutcome.Failed]: createThrowErrorHandler(fileSystem),
+           [TaskProcessingOutcome.Failed]:
+               errorOnFailedEntryPoint ? createThrowErrorHandler(fileSystem) :
+                                         createLogErrorHandler(logger, fileSystem, taskQueue),
          });
 }
 

packages/compiler-cli/ngcc/test/execution/tasks/queues/parallel_task_queue_spec.ts

@@ -9,6 +9,7 @@
 import {PartiallyOrderedTasks, TaskQueue} from '../../../../src/execution/tasks/api';
 import {ParallelTaskQueue} from '../../../../src/execution/tasks/queues/parallel_task_queue';
 import {computeTaskDependencies} from '../../../../src/execution/tasks/utils';
+import {MockLogger} from '../../../helpers/mock_logger';
 import {createTasksAndGraph} from '../../helpers';
 
 describe('ParallelTaskQueue', () => {
@@ -36,7 +37,7 @@ describe('ParallelTaskQueue', () => {
     const {tasks, graph} =
         createTasksAndGraph(entryPointCount, tasksPerEntryPointCount, entryPointDeps);
     const dependencies = computeTaskDependencies(tasks, graph);
-    return {tasks, queue: new ParallelTaskQueue(tasks.slice(), dependencies)};
+    return {tasks, queue: new ParallelTaskQueue(new MockLogger(), tasks.slice(), dependencies)};
   }
 
   /**

packages/compiler-cli/ngcc/test/execution/tasks/queues/serial_task_queue_spec.ts

@@ -11,6 +11,7 @@ import {PartiallyOrderedTasks, Task, TaskQueue} from '../../../../src/execution/
 import {SerialTaskQueue} from '../../../../src/execution/tasks/queues/serial_task_queue';
 import {computeTaskDependencies} from '../../../../src/execution/tasks/utils';
 import {EntryPoint} from '../../../../src/packages/entry_point';
+import {MockLogger} from '../../../helpers/mock_logger';
 
 
 describe('SerialTaskQueue', () => {
@@ -38,7 +39,7 @@ describe('SerialTaskQueue', () => {
       graph.addNode(entryPoint.path);
     }
     const dependencies = computeTaskDependencies(tasks, graph);
-    return {tasks, queue: new SerialTaskQueue(tasks.slice(), dependencies)};
+    return {tasks, queue: new SerialTaskQueue(new MockLogger(), tasks.slice(), dependencies)};
   };
 
   /**

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -1085,45 +1085,130 @@ runInEachFileSystem(() => {
     });
 
     describe('diagnostics', () => {
-      it('should fail with formatted diagnostics when an error diagnostic is produced', () => {
+      it('should fail with formatted diagnostics when an error diagnostic is produced, if targetEntryPointPath is provided',
-        loadTestFiles([
+         () => {
-          {
+           loadTestFiles([
-            name: _('/node_modules/fatal-error/package.json'),
+             {
-            contents: '{"name": "fatal-error", "es2015": "./index.js", "typings": "./index.d.ts"}',
+               name: _('/node_modules/fatal-error/package.json'),
-          },
+               contents:
-          {name: _('/node_modules/fatal-error/index.metadata.json'), contents: 'DUMMY DATA'},
+                   '{"name": "fatal-error", "es2015": "./index.js", "typings": "./index.d.ts"}',
-          {
+             },
-            name: _('/node_modules/fatal-error/index.js'),
+             {name: _('/node_modules/fatal-error/index.metadata.json'), contents: 'DUMMY DATA'},
-            contents: `
+             {
+               name: _('/node_modules/fatal-error/index.js'),
+               contents: `
               import {Component} from '@angular/core';
               export class FatalError {}
               FatalError.decorators = [
                 {type: Component, args: [{selector: 'fatal-error'}]}
               ];
             `,
-          },
+             },
-          {
+             {
-            name: _('/node_modules/fatal-error/index.d.ts'),
+               name: _('/node_modules/fatal-error/index.d.ts'),
-            contents: `
+               contents: `
               export declare class FatalError {}
             `,
-          },
+             },
-        ]);
+           ]);
 
-        try {
+           try {
-          mainNgcc({
+             mainNgcc({
-            basePath: '/node_modules',
+               basePath: '/node_modules',
-            targetEntryPointPath: 'fatal-error',
+               targetEntryPointPath: 'fatal-error',
-            propertiesToConsider: ['es2015']
+               propertiesToConsider: ['es2015']
-          });
+             });
-          fail('should have thrown');
+             fail('should have thrown');
-        } catch (e) {
+           } catch (e) {
-          expect(e.message).toContain(
+             expect(e.message).toContain(
-              'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:');
+                 'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:');
-          expect(e.message).toContain('NG2001');
+             expect(e.message).toContain('NG2001');
-          expect(e.message).toContain('component is missing a template');
+             expect(e.message).toContain('component is missing a template');
-        }
+           }
-      });
+         });
+
+      it('should not fail but log an error with formatted diagnostics when an error diagnostic is produced, if targetEntryPoint is not provided and errorOnFailedEntryPoint is false',
+         () => {
+           loadTestFiles([
+             {
+               name: _('/node_modules/fatal-error/package.json'),
+               contents:
+                   '{"name": "fatal-error", "es2015": "./index.js", "typings": "./index.d.ts"}',
+             },
+             {name: _('/node_modules/fatal-error/index.metadata.json'), contents: 'DUMMY DATA'},
+             {
+               name: _('/node_modules/fatal-error/index.js'),
+               contents: `
+             import {Component} from '@angular/core';
+             export class FatalError {}
+             FatalError.decorators = [
+               {type: Component, args: [{selector: 'fatal-error'}]}
+             ];`,
+             },
+             {
+               name: _('/node_modules/fatal-error/index.d.ts'),
+               contents: `export declare class FatalError {}`,
+             },
+             {
+               name: _('/node_modules/dependent/package.json'),
+               contents: '{"name": "dependent", "es2015": "./index.js", "typings": "./index.d.ts"}',
+             },
+             {name: _('/node_modules/dependent/index.metadata.json'), contents: 'DUMMY DATA'},
+             {
+               name: _('/node_modules/dependent/index.js'),
+               contents: `
+             import {Component} from '@angular/core';
+             import {FatalError} from 'fatal-error';
+             export class Dependent {}
+             Dependent.decorators = [
+               {type: Component, args: [{selector: 'dependent', template: ''}]}
+             ];`,
+             },
+             {
+               name: _('/node_modules/dependent/index.d.ts'),
+               contents: `export declare class Dependent {}`,
+             },
+             {
+               name: _('/node_modules/independent/package.json'),
+               contents:
+                   '{"name": "independent", "es2015": "./index.js", "typings": "./index.d.ts"}',
+             },
+             {name: _('/node_modules/independent/index.metadata.json'), contents: 'DUMMY DATA'},
+             {
+               name: _('/node_modules/independent/index.js'),
+               contents: `
+             import {Component} from '@angular/core';
+             export class Independent {}
+             Independent.decorators = [
+               {type: Component, args: [{selector: 'independent', template: ''}]}
+             ];`,
+             },
+             {
+               name: _('/node_modules/independent/index.d.ts'),
+               contents: `export declare class Independent {}`,
+             },
+           ]);
+
+           const logger = new MockLogger();
+           mainNgcc({
+             basePath: '/node_modules',
+             propertiesToConsider: ['es2015'],
+             errorOnFailedEntryPoint: false, logger,
+           });
+           expect(logger.logs.error.length).toEqual(1);
+           const message = logger.logs.error[0][0];
+           expect(message).toContain(
+               'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:');
+           expect(message).toContain('NG2001');
+           expect(message).toContain('component is missing a template');
+
+           expect(hasBeenProcessed(loadPackage('fatal-error', _('/node_modules')), 'es2015'))
+               .toBe(false);
+           expect(hasBeenProcessed(loadPackage('dependent', _('/node_modules')), 'es2015'))
+               .toBe(false);
+           expect(hasBeenProcessed(loadPackage('independent', _('/node_modules')), 'es2015'))
+               .toBe(true);
+         });
     });
 
     describe('logger', () => {