refactor(travisclient): make changes to travisClient to use Spinnaker…

… custom exception instead of RetrofitError using SpinnakerRetrofitErrorHandler (#1090) * refactor(travisService/test): replace RetrofitError with SpinnakerRetrofitErrorHandler for custom error handling in travis client * refactor(travisService/test): added null check to response body refactor(travisService/test): response body could be null for eg when a json converter is unable to convert non-json format * refactor(travisService): remove try-catch since e.getBodyAs() is no longer called --------- Co-authored-by: Sheetal Atre <[email protected]>
spinnaker · Sep 14, 2023 · 1c88412 · 1c88412
1 parent ae0d3d5
commit 1c88412
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 23 deletions.
diff --git a/igor-monitor-travis/igor-monitor-travis.gradle b/igor-monitor-travis/igor-monitor-travis.gradle
@@ -23,6 +23,7 @@ dependencies {
   implementation "io.spinnaker.kork:kork-jedis"
   implementation "io.spinnaker.kork:kork-security"
   implementation "io.spinnaker.kork:kork-web"
+  implementation "io.spinnaker.kork:kork-retrofit"
 
   implementation "com.fasterxml.jackson.core:jackson-databind"
   implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-xml"

diff --git a/igor-monitor-travis/src/main/java/com/netflix/spinnaker/igor/travis/config/TravisConfig.java b/igor-monitor-travis/src/main/java/com/netflix/spinnaker/igor/travis/config/TravisConfig.java
@@ -25,6 +25,7 @@
 import com.netflix.spinnaker.igor.travis.client.TravisClient;
 import com.netflix.spinnaker.igor.travis.client.model.v3.Root;
 import com.netflix.spinnaker.igor.travis.service.TravisService;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
 import com.squareup.okhttp.OkHttpClient;
 import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry;
@@ -132,6 +133,7 @@ public static TravisClient travisClient(String address, int timeout, ObjectMappe
         .setClient(new OkClient(client))
         .setConverter(new JacksonConverter(objectMapper))
         .setLog(new Slf4jRetrofitLogger(TravisClient.class))
+        .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
         .build()
         .create(TravisClient.class);
   }

diff --git a/...monitor-travis/src/main/java/com/netflix/spinnaker/igor/travis/service/TravisService.java b/...monitor-travis/src/main/java/com/netflix/spinnaker/igor/travis/service/TravisService.java
@@ -50,6 +50,8 @@
 import com.netflix.spinnaker.igor.travis.client.model.v3.V3Builds;
 import com.netflix.spinnaker.igor.travis.client.model.v3.V3Job;
 import com.netflix.spinnaker.igor.travis.client.model.v3.V3Log;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
 import io.github.resilience4j.circuitbreaker.CircuitBreaker;
 import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry;
 import io.github.resilience4j.core.SupplierUtils;
@@ -59,7 +61,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -73,7 +74,6 @@
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import retrofit.RetrofitError;
 
 public class TravisService implements BuildOperations, BuildProperties {
 
@@ -444,27 +444,21 @@ private Optional<String> getAndCacheJobLog(int jobId) {
         travisCache.setJobLog(groupKey, jobId, v3Log.getContent());
         return Optional.of(v3Log.getContent());
       }
-    } catch (RetrofitError e) {
-      if (e.getBody() != null) {
-        try {
-          Map body = (Map) e.getBodyAs(HashMap.class);
-          if ("log_expired".equals(body.get("error_type"))) {
-            log.info(
-                "{}: The log for job id {} has expired and the corresponding build was ignored",
-                groupKey,
-                jobId);
-          } else {
-            log.warn(
-                "{}: Could not get log for job id {}. Error from Travis:\n{}",
-                groupKey,
-                jobId,
-                body);
-          }
-        } catch (RuntimeException ex) {
-          log.warn("{}: Could not parse original error message from Travis", groupKey, ex);
+    } catch (SpinnakerServerException e) {
+      if (e instanceof SpinnakerHttpException) { // only SpinnakerHttpException has a response body
+        Map<String, Object> body = ((SpinnakerHttpException) e).getResponseBody();
+        if (body != null && "log_expired".equals(body.get("error_type"))) {
+          log.info(
+              "{}: The log for job id {} has expired and the corresponding build was ignored",
+              groupKey,
+              jobId);
+        } else {
+          log.warn(
+              "{}: Could not get log for job id {}. Error from Travis:\n{}", groupKey, jobId, body);
         }
       }
     }
+
     return Optional.empty();
   }
 
@@ -553,7 +547,7 @@ public Map<String, Integer> queuedBuild(String master, int queueId) {
   public void syncRepos() {
     try {
       travisClient.usersSync(getAccessToken(), new EmptyObject());
-    } catch (RetrofitError e) {
+    } catch (SpinnakerServerException e) {
       log.error(
           "synchronizing travis repositories for {} failed with error: {}",
           groupKey,

diff --git a/...travis/src/test/groovy/com/netflix/spinnaker/igor/travis/service/TravisServiceSpec.groovy b/...travis/src/test/groovy/com/netflix/spinnaker/igor/travis/service/TravisServiceSpec.groovy
@@ -37,6 +37,7 @@ import com.netflix.spinnaker.igor.travis.client.model.v3.V3Job
 import com.netflix.spinnaker.igor.travis.client.model.v3.V3Jobs
 import com.netflix.spinnaker.igor.travis.client.model.v3.V3Log
 import com.netflix.spinnaker.igor.travis.client.model.v3.V3Repository
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry
 import org.assertj.core.util.Lists
 import retrofit.RetrofitError
@@ -391,7 +392,7 @@ class TravisServiceSpec extends Specification {
         2 * travisCache.getJobLog("travis-ci", _) >> null
         1 * client.jobLog(_, 1) >> v3log
         1 * client.jobLog(_, 2) >> {
-            throw RetrofitError.httpError(
+            throw new SpinnakerHttpException(RetrofitError.httpError(
                 "https://travis-ci.com/api/job/2/log",
                 new Response("https://travis-ci.com/api/job/2/log", 403, "Forbidden", [], new TypedString(
                     """{
@@ -401,7 +402,7 @@ class TravisServiceSpec extends Specification {
                         }
                     """)),
                 new JacksonConverter(),
-                Map)
+                Map))
         }
         !ready
     }