commit 238839cd79ac7bef427df7e6604992b02eff05b7
author Philipp Wollermann <philwo@google.com> Thu Oct 27 08:55:07 2016 +0000
committer Laszlo Csomor <laszlocsomor@google.com> Thu Oct 27 09:28:23 2016 +0000
tree a84e457f66e36836455787d488e76450118efb2b
parent c1fb834c4189d47d0fbdcf6c351821a1841d04e0

Refactor exception handling in SandboxStrategy.

--
MOS_MIGRATED_REVID=137370890

src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
index 77334e9..b91d3dd 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
@@ -23,6 +23,7 @@
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
 import com.google.devtools.build.lib.actions.Spawns;
+import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.events.Event;
@@ -67,6 +68,7 @@
       AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles)
       throws ExecException, InterruptedException {
     EventHandler eventHandler = actionExecutionContext.getExecutor().getEventHandler();
+    ExecException execException = null;
     try {
       runner.run(
           spawn.getArguments(),
@@ -74,29 +76,33 @@
           actionExecutionContext.getFileOutErr(),
           Spawns.getTimeoutSeconds(spawn),
           SandboxHelpers.shouldAllowNetwork(buildRequest, spawn));
-    } finally {
-      if (writeOutputFiles != null
-          && !writeOutputFiles.compareAndSet(null, SandboxStrategy.class)) {
-        Thread.currentThread().interrupt();
+    } catch (ExecException e) {
+      execException = e;
+    }
+
+    if (writeOutputFiles != null && !writeOutputFiles.compareAndSet(null, SandboxStrategy.class)) {
+      throw new InterruptedException();
+    }
+
+    try {
+      // We copy the outputs even when the command failed, otherwise StandaloneTestStrategy
+      // won't be able to get the test logs of a failed test. (We should probably do this in
+      // some better way.)
+      sandboxExecRoot.copyOutputs(execRoot, outputs);
+    } catch (IOException e) {
+      if (execException == null) {
+        throw new UserExecException("Could not move output artifacts from sandboxed execution", e);
       } else {
-        try {
-          // We copy the outputs even when the command failed, otherwise StandaloneTestStrategy
-          // won't be able to get the test logs of a failed test. (We should probably do this in
-          // some better way.)
-          sandboxExecRoot.copyOutputs(execRoot, outputs);
-        } catch (IOException e) {
-          // Catch the IOException and turn it into an error message, otherwise this might hide an
-          // exception thrown during runner.run earlier.
-          eventHandler.handle(
-              Event.error(
-                  "I/O exception while extracting output artifacts from sandboxed execution: "
-                      + e));
-        }
+        // Catch the IOException and turn it into an error message, otherwise this might hide an
+        // exception thrown during runner.run earlier.
+        eventHandler.handle(
+            Event.error(
+                "I/O exception while extracting output artifacts from sandboxed execution: " + e));
       }
     }
 
-    if (Thread.interrupted()) {
-      throw new InterruptedException();
+    if (execException != null) {
+      throw execException;
     }
   }