refactor(JenkinsClient): make changes to jenkinsclient to use Spinnak…

…er custom exception instead of RetrofitError using SpinnakerRetrofitErrorHandler
spinnaker · Aug 29, 2023 · 20a4d6c · 20a4d6c
1 parent e44b4df
commit 20a4d6c
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 45 deletions.
diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/build/BuildController.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/build/BuildController.groovy
@@ -47,7 +47,6 @@ import org.springframework.web.bind.annotation.RequestMethod
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.RestController
 import org.springframework.web.servlet.HandlerMapping
-import retrofit.RetrofitError
 import retrofit.http.Query
 
 import javax.annotation.Nullable
@@ -181,11 +180,7 @@ class BuildController {
             buildService.stopQueuedBuild(queuedBuild)
           }
         } catch (SpinnakerServerException e) {
-          if (e instanceof SpinnakerHttpException && ((SpinnakerHttpException) e).responseCode != NOT_FOUND.value()) {
-            throw e
-          }
-        } catch (RetrofitError e) {
-          if (e.response?.status != NOT_FOUND.value()) {
+          if (e instanceof SpinnakerHttpException && ((SpinnakerHttpException) e).getResponseCode() != NOT_FOUND.value()) {
             throw e
           }
         }

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsService.java b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsService.java
@@ -95,10 +95,10 @@ public JenkinsService(
             CircuitBreakerConfig.custom()
                 .ignoreException(
                     (e) -> {
-                      if (e instanceof RetrofitError) {
-                        RetrofitError re = (RetrofitError) e;
-                        return re.getKind() == RetrofitError.Kind.HTTP
-                            && re.getResponse().getStatus() == 404;
+                      if (e instanceof SpinnakerServerException) {
+                        SpinnakerServerException re = (SpinnakerServerException) e;
+                        return re instanceof SpinnakerHttpException
+                            && ((SpinnakerHttpException) re).getResponseCode() == 404;
                       }
                       return false;
                     })
@@ -218,16 +218,6 @@ private ScmDetails getGitDetails(String jobName, Integer buildNumber) {
         () -> {
           try {
             return jenkinsClient.getGitDetails(encode(jobName), buildNumber);
-          } catch (SpinnakerServerException e) {
-            // assuming that a conversion error is unlikely to succeed on retry
-            if (e instanceof SpinnakerConversionException) {
-              log.warn(
-                  "Unable to deserialize git details for build " + buildNumber + " of " + jobName,
-                  e);
-              return null;
-            } else {
-              throw e;
-            }
           } catch (RetrofitError e) {
             // assuming that a conversion error is unlikely to succeed on retry
             if (e.getKind() == RetrofitError.Kind.CONVERSION) {
@@ -260,12 +250,6 @@ public QueuedJob queuedBuild(String master, int item) {
             String.format("Queued job '%s' not found for master '%s'.", item, master));
       }
       throw e;
-    } catch (RetrofitError e) {
-      if (e.getResponse() != null && e.getResponse().getStatus() == NOT_FOUND.value()) {
-        throw new NotFoundException(
-            String.format("Queued job '%s' not found for master '%s'.", item, master));
-      }
-      throw e;
     }
   }
 
@@ -351,25 +335,12 @@ private Response getPropertyFile(String jobName, Integer buildNumber, String fil
         () -> {
           try {
             return jenkinsClient.getPropertyFile(encode(jobName), buildNumber, fileName);
-          } catch (SpinnakerNetworkException e) {
-            throw e;
-          } catch (SpinnakerHttpException e) {
-            if (e.getResponseCode() == 404 || e.getResponseCode() >= 500) {
-              throw e;
-            }
-            SpinnakerException ex = new SpinnakerException(e);
-            ex.setRetryable(false);
-            throw ex;
           } catch (SpinnakerServerException e) {
-            SpinnakerException ex = new SpinnakerException(e);
-            ex.setRetryable(false);
-            throw ex;
-          } catch (RetrofitError e) {
             // retry on network issue, 404 and 5XX
-            if (e.getKind() == RetrofitError.Kind.NETWORK
-                || (e.getKind() == RetrofitError.Kind.HTTP
-                    && (e.getResponse().getStatus() == 404
-                        || e.getResponse().getStatus() >= 500))) {
+            if (e instanceof SpinnakerNetworkException
+                || (e instanceof SpinnakerHttpException
+                    && (((SpinnakerHttpException) e).getResponseCode() == 404
+                        || ((SpinnakerHttpException) e).getResponseCode() >= 500))) {
               throw e;
             }
             SpinnakerException ex = new SpinnakerException(e);

diff --git a/...-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsServiceSpec.groovy b/...-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsServiceSpec.groovy
@@ -26,6 +26,7 @@ import com.netflix.spinnaker.igor.jenkins.client.model.Build
 import com.netflix.spinnaker.igor.jenkins.client.model.BuildArtifact
 import com.netflix.spinnaker.igor.jenkins.client.model.BuildsList
 import com.netflix.spinnaker.igor.jenkins.client.model.Project
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.squareup.okhttp.mockwebserver.MockResponse
 import com.squareup.okhttp.mockwebserver.MockWebServer
 import io.github.resilience4j.circuitbreaker.CircuitBreaker
@@ -765,12 +766,12 @@ class JenkinsServiceSpec extends Specification {
       build.number = buildNumber
       build.duration = 0L
       build.artifacts = [artifact]
-      def badRequestError = RetrofitError.httpError(
+      def badRequestError = new SpinnakerHttpException(RetrofitError.httpError(
         "http://my.jenkins.net",
         new Response("http://my.jenkins.net", 400, "bad request", [], null),
         null,
         null
-      )
+      ))
 
       when:
       def properties = service.getBuildProperties(jobName, build.genericBuild(jobName), artifact.fileName)