feat(core): cache task failures

Author: vsavkin

e2e/workspace/src/workspace.test.ts

@@ -675,6 +675,30 @@ describe('cache', () => {
       `${myapp2}-e2e`,
     ]);
 
+    // cache task failures
+    // --------------------------------------------
+    updateFile('workspace.json', (c) => {
+      const workspaceJson = JSON.parse(c);
+      workspaceJson.projects[myapp1].targets.lint = {
+        executor: '@nrwl/workspace:run-commands',
+        options: {
+          command: 'echo hi && exit 1',
+        },
+      };
+      return JSON.stringify(workspaceJson, null, 2);
+    });
+    const failingRun = runCLI(`lint ${myapp1}`, {
+      silenceError: true,
+      env: { ...process.env, NX_CACHE_FAILURES: 'true' },
+    });
+    expect(failingRun).not.toContain('[retrieved from cache]');
+
+    const cachedFailingRun = runCLI(`lint ${myapp1}`, {
+      silenceError: true,
+      env: { ...process.env, NX_CACHE_FAILURES: 'true' },
+    });
+    expect(cachedFailingRun).toContain('[retrieved from cache]');
+
     // run without caching
     // --------------------------------------------
 

packages/workspace/src/tasks-runner/cache.ts

@@ -13,7 +13,11 @@ import { DefaultTasksRunnerOptions } from './default-tasks-runner';
 import { spawn } from 'child_process';
 import { cacheDirectory } from '../utilities/cache-directory';
 
-export type CachedResult = { terminalOutput: string; outputsPath: string };
+export type CachedResult = {
+  terminalOutput: string;
+  outputsPath: string;
+  code: number;
+};
 export type TaskWithCachedResult = { task: Task; cachedResult: CachedResult };
 
 class CacheConfig {
@@ -83,8 +87,12 @@ export class Cache {
     }
   }
 
-  async put(task: Task, terminalOutputPath: string, outputs: string[]) {
-    const terminalOutput = readFileSync(terminalOutputPath).toString();
+  async put(
+    task: Task,
+    terminalOutput: string | null,
+    outputs: string[],
+    code: number
+  ) {
     const td = join(this.cachePath, task.hash);
     const tdCommit = join(this.cachePath, `${task.hash}.commit`);
 
@@ -97,7 +105,10 @@ export class Cache {
     }
 
     mkdirSync(td);
-    writeFileSync(join(td, 'terminalOutput'), terminalOutput);
+    writeFileSync(
+      join(td, 'terminalOutput'),
+      terminalOutput ?? 'no terminal output'
+    );
 
     mkdirSync(join(td, 'outputs'));
     outputs.forEach((f) => {
@@ -116,14 +127,19 @@ export class Cache {
     // creating this file is atomic, whereas creating a folder is not.
     // so if the process gets terminated while we are copying stuff into cache,
     // the cache entry won't be used.
+    writeFileSync(join(td, 'code'), code.toString());
     writeFileSync(tdCommit, 'true');
 
     if (this.options.remoteCache) {
       await this.options.remoteCache.store(task.hash, this.cachePath);
     }
   }
 
-  copyFilesFromCache(cachedResult: CachedResult, outputs: string[]) {
+  copyFilesFromCache(
+    hash: string,
+    cachedResult: CachedResult,
+    outputs: string[]
+  ) {
     outputs.forEach((f) => {
       const cached = join(cachedResult.outputsPath, f);
       if (existsSync(cached)) {
@@ -153,9 +169,17 @@ export class Cache {
     const td = join(this.cachePath, task.hash);
 
     if (existsSync(tdCommit)) {
+      const terminalOutput = readFileSync(
+        join(td, 'terminalOutput')
+      ).toString();
+      let code = 0;
+      try {
+        code = Number(readFileSync(join(td, 'code')).toString());
+      } catch (e) {}
       return {
-        terminalOutput: readFileSync(join(td, 'terminalOutput')).toString(),
+        terminalOutput,
         outputsPath: join(td, 'outputs'),
+        code,
       };
     } else {
       return null;

packages/workspace/src/tasks-runner/task-orchestrator.ts

@@ -125,16 +125,16 @@ export class TaskOrchestrator {
       }
 
       const outputs = getOutputs(this.projectGraph.nodes, t.task);
-      this.cache.copyFilesFromCache(t.cachedResult, outputs);
+      this.cache.copyFilesFromCache(t.task.hash, t.cachedResult, outputs);
 
-      this.options.lifeCycle.endTask(t.task, 0);
+      this.options.lifeCycle.endTask(t.task, t.cachedResult.code);
     });
 
-    return tasks.reduce((m, c) => {
+    return tasks.reduce((m, t) => {
       m.push({
-        task: c.task,
+        task: t.task,
         type: AffectedEventType.TaskCacheRead,
-        success: true,
+        success: t.cachedResult.code === 0,
       });
       return m;
     }, []);
@@ -205,10 +205,11 @@ export class TaskOrchestrator {
             process.stdout.write(outWithErr.join(''));
           }
           if (outputPath) {
-            fs.writeFileSync(outputPath, outWithErr.join(''));
-            if (code === 0) {
+            const terminalOutput = outWithErr.join('');
+            fs.writeFileSync(outputPath, terminalOutput);
+            if (this.shouldCacheTask(outputPath, code)) {
               this.cache
-                .put(task, outputPath, taskOutputs)
+                .put(task, terminalOutput, taskOutputs, code)
                 .then(() => {
                   this.options.lifeCycle.endTask(task, code);
                   res(code);
@@ -259,21 +260,22 @@ export class TaskOrchestrator {
         p.on('exit', (code) => {
           if (code === null) code = 2;
           // we didn't print any output as we were running the command
-          // print all the collected output|
+          // print all the collected output
           if (!forwardOutput) {
             output.logCommand(commandLine);
-            try {
-              process.stdout.write(fs.readFileSync(outputPath));
-            } catch (e) {
+            const terminalOutput = this.readTerminalOutput(outputPath);
+            if (terminalOutput) {
+              process.stdout.write(terminalOutput);
+            } else {
               console.error(
                 `Nx could not find process's output. Run the command without --parallel.`
               );
             }
           }
           // we don't have to worry about this statement. code === 0 guarantees the file is there.
-          if (outputPath && code === 0) {
+          if (this.shouldCacheTask(outputPath, code)) {
             this.cache
-              .put(task, outputPath, taskOutputs)
+              .put(task, this.readTerminalOutput(outputPath), taskOutputs, code)
               .then(() => {
                 this.options.lifeCycle.endTask(task, code);
                 res(code);
@@ -293,6 +295,23 @@ export class TaskOrchestrator {
     });
   }
 
+  private readTerminalOutput(outputPath: string) {
+    try {
+      return fs.readFileSync(outputPath).toString();
+    } catch (e) {
+      return null;
+    }
+  }
+
+  private shouldCacheTask(outputPath: string | null, code: number) {
+    // TODO: vsavkin make caching failures the default in Nx 12.1
+    if (process.env.NX_CACHE_FAILURES == 'true') {
+      return outputPath;
+    } else {
+      return outputPath && code === 0;
+    }
+  }
+
   private envForForkedProcess(
     task: Task,
     outputPath: string,