Improve retry strategy for Google cloud errors when writing task help…

…er files (#5037)



Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

modules/nextflow/src/main/groovy/nextflow/executor/BashWrapperBuilder.groovy

@@ -449,7 +449,9 @@ class BashWrapperBuilder {
                 }
                 return path
             }
-            catch (FileSystemException | SocketException | RuntimeException e) {
+            catch (Exception e) {
+                if( !isRetryable0(e) )
+                    throw e
                 final isLocalFS = path.getFileSystem()==FileSystems.default
                 // the retry logic is needed for non-local file system such as S3.
                 // when the file is local fail without retrying
@@ -463,6 +465,18 @@ class BashWrapperBuilder {
         }
     }
 
+    static protected boolean isRetryable0(Exception e) {
+        if( e instanceof FileSystemException )
+            return true
+        if( e instanceof SocketException )
+            return true
+        if( e instanceof RuntimeException )
+            return true
+        if( e.class.getSimpleName() == 'HttpResponseException' )
+            return true
+        return false
+    }
+
     protected String getTaskMetadata() {
         final lines = new StringBuilder()
         lines << '### ---\n'

modules/nextflow/src/test/groovy/nextflow/executor/BashWrapperBuilderTest.groovy

@@ -16,6 +16,7 @@
 
 package nextflow.executor
 
+import java.nio.file.FileSystemException
 import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.Paths
@@ -1359,4 +1360,17 @@ class BashWrapperBuilderTest extends Specification {
         binding.cleanup_cmd == 'rm -rf $NXF_SCRATCH || true\npodman rm $NXF_BOXID &>/dev/null || true\n'
         binding.kill_cmd == 'podman stop $NXF_BOXID'
     }
+
+    @Unroll
+    def 'should check retryable errors' () {
+        expect:
+        BashWrapperBuilder.isRetryable0(ERROR) == EXPECTED
+        where:
+        ERROR                           | EXPECTED
+        new RuntimeException()          | true
+        new SocketException()           | true
+        new FileSystemException('foo')  | true
+        new IOException()               | false
+        new Exception()                 | false
+    }
 }

plugins/nf-google/src/test/nextflow/executor/BashWrapperBuilderWithGoogleTest.groovy

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2013-2024, Seqera Labs
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package nextflow.executor
+
+
+import com.google.api.client.http.HttpResponseException
+import spock.lang.Specification
+/**
+ *
+ * @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
+ */
+class BashWrapperBuilderWithGoogleTest extends Specification {
+
+    def 'should check retryable errors' () {
+        expect:
+        BashWrapperBuilder.isRetryable0(ERROR) == EXPECTED
+        where:
+        ERROR                                                                   | EXPECTED
+        new HttpResponseException(GroovyMock(HttpResponseException.Builder))    | true
+        new Exception()                                                         | false
+    }
+
+}