fix(ngcc): ensure lockfile is removed when analyzeFn fails (#37739)

Previously an error thrown in the `analyzeFn` would cause
the ngcc process to exit immediately without removing the
lockfile, and potentially before the unlocker process had been
successfully spawned resulting in the lockfile being orphaned
and left behind.

Now we catch these errors and remove the lockfile as needed.

PR Close #37739

packages/compiler-cli/ngcc/src/execution/cluster/executor.ts

@@ -28,13 +28,13 @@ export class ClusterExecutor implements Executor {
 
   async execute(analyzeEntryPoints: AnalyzeEntryPointsFn, _createCompileFn: CreateCompileFn):
       Promise<void> {
-    return this.lockFile.lock(() => {
+    return this.lockFile.lock(async () => {
       this.logger.debug(
           `Running ngcc on ${this.constructor.name} (using ${this.workerCount} worker processes).`);
       const master = new ClusterMaster(
           this.workerCount, this.fileSystem, this.logger, this.fileWriter, this.pkgJsonUpdater,
           analyzeEntryPoints, this.createTaskCompletedCallback);
-      return master.run();
+      return await master.run();
     });
   }
 }

packages/compiler-cli/ngcc/src/locking/async_locker.ts

@@ -37,7 +37,11 @@ export class AsyncLocker {
    */
   async lock<T>(fn: () => Promise<T>): Promise<T> {
     await this.create();
-    return fn().finally(() => this.lockFile.remove());
+    try {
+      return await fn();
+    } finally {
+      this.lockFile.remove();
+    }
   }
 
   protected async create() {

packages/compiler-cli/ngcc/test/execution/cluster/executor_spec.ts

@@ -34,7 +34,7 @@ runInEachFileSystem(() => {
 
     beforeEach(() => {
       masterRunSpy = spyOn(ClusterMaster.prototype, 'run')
-                         .and.returnValue(Promise.resolve('CusterMaster#run()' as any));
+                         .and.returnValue(Promise.resolve('ClusterMaster#run()' as any));
       createTaskCompletedCallback = jasmine.createSpy('createTaskCompletedCallback');
 
       mockLogger = new MockLogger();
@@ -63,7 +63,7 @@ runInEachFileSystem(() => {
         const createCompilerFnSpy = jasmine.createSpy('createCompilerFn');
 
         expect(await executor.execute(analyzeEntryPointsSpy, createCompilerFnSpy))
-            .toBe('CusterMaster#run()' as any);
+            .toBe('ClusterMaster#run()' as any);
 
         expect(masterRunSpy).toHaveBeenCalledWith();
 
@@ -78,6 +78,22 @@ runInEachFileSystem(() => {
            expect(lockFileLog).toEqual(['write()', 'remove()']);
          });
 
+      it('should call LockFile.write() and LockFile.remove() if analyzeFn fails', async () => {
+        const analyzeEntryPointsSpy =
+            jasmine.createSpy('analyzeEntryPoints').and.throwError('analyze error');
+        const createCompilerFnSpy = jasmine.createSpy('createCompilerFn');
+        let error = '';
+        try {
+          await executor.execute(analyzeEntryPointsSpy, createCompilerFnSpy);
+        } catch (e) {
+          error = e.message;
+        }
+        expect(analyzeEntryPointsSpy).toHaveBeenCalledWith();
+        expect(createCompilerFnSpy).not.toHaveBeenCalled();
+        expect(error).toEqual('analyze error');
+        expect(lockFileLog).toEqual(['write()', 'remove()']);
+      });
+
       it('should call LockFile.write() and LockFile.remove() if master runner fails', async () => {
         const anyFn: () => any = () => undefined;
         masterRunSpy.and.returnValue(Promise.reject(new Error('master runner error')));