feat(retrofit): move url attribute from SpinnakerHttpException to SpinnakerServerException

kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/ErrorHandlingExecutorCallAdapterFactory.java

@@ -152,11 +152,12 @@ public Response<T> execute() {
           return syncResp;
         }
       } catch (JsonProcessingException jpe) {
-        throw new SpinnakerConversionException("Failed to process response body", jpe);
+        throw new SpinnakerConversionException(
+            "Failed to process response body", jpe, delegate.request().url().toString());
       } catch (IOException e) {
-        throw new SpinnakerNetworkException(e);
+        throw new SpinnakerNetworkException(e, delegate.request().url().toString());
       } catch (Exception e) {
-        throw new SpinnakerServerException(e);
+        throw new SpinnakerServerException(e, delegate.request().url().toString());
       }
       throw new SpinnakerHttpException(syncResp, retrofit);
     }
@@ -250,11 +251,11 @@ public void onFailure(Call<T> call, final Throwable t) {
 
       SpinnakerServerException exception;
       if (t instanceof IOException) {
-        exception = new SpinnakerNetworkException(t);
+        exception = new SpinnakerNetworkException(t, call.request().url().toString());
       } else if (t instanceof SpinnakerHttpException) {
         exception = (SpinnakerHttpException) t;
       } else {
-        exception = new SpinnakerServerException(t);
+        exception = new SpinnakerServerException(t, call.request().url().toString());
       }
       final SpinnakerServerException finalException = exception;
       callbackExecutor.execute(

kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2SyncCall.java

@@ -33,7 +33,7 @@ public static <T> T execute(Call<T> call) {
     try {
       return call.execute().body();
     } catch (IOException e) {
-      throw new SpinnakerNetworkException(e);
+      throw new SpinnakerNetworkException(e, call.request().url().toString());
     }
   }
 }

kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerConversionException.java

@@ -16,14 +16,24 @@
 
 package com.netflix.spinnaker.kork.retrofit.exceptions;
 
+import retrofit.RetrofitError;
+
 /** Wraps an exception converting a successful retrofit http response body to its indicated type */
 public class SpinnakerConversionException extends SpinnakerServerException {
 
-  public SpinnakerConversionException(String message, Throwable cause) {
-    super(message, cause);
+  public SpinnakerConversionException(String message, Throwable cause, String url) {
+    super(message, cause, url);
     setRetryable(false);
   }
 
+  public SpinnakerConversionException(String message, SpinnakerConversionException cause) {
+    super(message, cause);
+  }
+
+  public SpinnakerConversionException(RetrofitError e) {
+    super(e);
+  }
+
   @Override
   public SpinnakerConversionException newInstance(String message) {
     return new SpinnakerConversionException(message, this);

kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java

@@ -56,8 +56,6 @@ public class SpinnakerHttpException extends SpinnakerServerException {
 
   private final Map<String, Object> responseBody;
 
-  private final String url;
-
   private final int responseCode;
 
   /**
@@ -101,7 +99,6 @@ public SpinnakerHttpException(RetrofitError e) {
     if (responseBody != null) {
       tmpMessage = (String) responseBody.get("message");
     }
-    url = e.getUrl();
     responseCode = response.getStatus();
     reason = response.getReason();
     rawMessage = tmpMessage != null ? tmpMessage : reason;
@@ -113,14 +110,14 @@ public SpinnakerHttpException(RetrofitError e) {
    */
   public SpinnakerHttpException(
       retrofit2.Response<?> retrofit2Response, retrofit2.Retrofit retrofit) {
+    super(retrofit2Response);
     this.response = null;
     this.retrofit2Response = retrofit2Response;
     if ((retrofit2Response.code() == HttpStatus.NOT_FOUND.value())
         || (retrofit2Response.code() == HttpStatus.BAD_REQUEST.value())) {
       setRetryable(false);
     }
     responseBody = this.getErrorBodyAs(retrofit);
-    url = retrofit2Response.raw().request().url().toString();
     responseCode = retrofit2Response.code();
     reason = retrofit2Response.message();
     this.rawMessage =
@@ -159,7 +156,6 @@ public SpinnakerHttpException(String message, SpinnakerHttpException cause) {
     this.retrofit2Response = cause.retrofit2Response;
     rawMessage = null;
     this.responseBody = cause.responseBody;
-    this.url = cause.url;
     this.responseCode = cause.responseCode;
     this.reason = cause.reason;
   }
@@ -197,7 +193,8 @@ public String getMessage() {
       return super.getMessage();
     }
 
-    return String.format("Status: %s, URL: %s, Message: %s", responseCode, url, getRawMessage());
+    return String.format(
+        "Status: %s, URL: %s, Message: %s", responseCode, this.getUrl(), getRawMessage());
   }
 
   @Override
@@ -209,10 +206,6 @@ public Map<String, Object> getResponseBody() {
     return this.responseBody;
   }
 
-  public String getUrl() {
-    return this.url;
-  }
-
   public String getReason() {
     return this.reason;
   }

kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerNetworkException.java

@@ -22,14 +22,18 @@
 /** Wraps an exception of kind {@link RetrofitError.Kind} NETWORK. */
 @NonnullByDefault
 public final class SpinnakerNetworkException extends SpinnakerServerException {
-  public SpinnakerNetworkException(Throwable cause) {
-    super(cause);
+  public SpinnakerNetworkException(Throwable cause, String url) {
+    super(cause, url);
   }
 
-  public SpinnakerNetworkException(String message, Throwable cause) {
+  public SpinnakerNetworkException(String message, SpinnakerNetworkException cause) {
     super(message, cause);
   }
 
+  public SpinnakerNetworkException(RetrofitError e) {
+    super(e);
+  }
+
   @Override
   public SpinnakerNetworkException newInstance(String message) {
     return new SpinnakerNetworkException(message, this);

kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandler.java

@@ -59,11 +59,11 @@ public Throwable handleError(RetrofitError e) {
         }
         return retval;
       case NETWORK:
-        return new SpinnakerNetworkException(e.getMessage(), e.getCause());
+        return new SpinnakerNetworkException(e);
       case CONVERSION:
-        return new SpinnakerConversionException(e.getMessage(), e.getCause());
+        return new SpinnakerConversionException(e);
       default:
-        return new SpinnakerServerException(e.getMessage(), e.getCause());
+        return new SpinnakerServerException(e);
     }
   }
 

kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerException.java

@@ -18,23 +18,46 @@
 
 import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
 import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
+import retrofit.RetrofitError;
+import retrofit2.Response;
 
 /** Represents an error while attempting to execute a retrofit http client request. */
 @NonnullByDefault
 public class SpinnakerServerException extends SpinnakerException {
 
-  public SpinnakerServerException(String message, Throwable cause) {
-    super(message, cause);
+  private final String url;
+
+  public SpinnakerServerException(RetrofitError e) {
+    super(e.getMessage(), e.getCause());
+    url = e.getUrl();
+  }
+
+  public SpinnakerServerException(Response retrofit2Response) {
+    super();
+    url = retrofit2Response.raw().request().url().toString();
   }
 
-  public SpinnakerServerException(Throwable cause) {
+  public SpinnakerServerException(Throwable cause, String url) {
     super(cause);
+    this.url = url;
   }
 
-  public SpinnakerServerException() {}
+  public SpinnakerServerException(String message, Throwable cause, String url) {
+    super(message, cause);
+    this.url = url;
+  }
+
+  public SpinnakerServerException(String message, SpinnakerServerException cause) {
+    super(message, cause);
+    this.url = cause.getUrl();
+  }
 
   @Override
   public SpinnakerServerException newInstance(String message) {
     return new SpinnakerServerException(message, this);
   }
+
+  public String getUrl() {
+    return this.url;
+  }
 }

kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/Retrofit2SyncCallTest.java

@@ -16,38 +16,45 @@
 
 package com.netflix.spinnaker.kork.retrofit.exceptions;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowableOfType;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
 import java.io.IOException;
+import okhttp3.HttpUrl;
+import okhttp3.Request;
 import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
 import retrofit2.Call;
 import retrofit2.Response;
 
-public class Retrofit2SyncCallTest {
+class Retrofit2SyncCallTest {
 
   @Test
-  public void testExecuteSuccss() throws IOException {
-    Call<String> mockcall = Mockito.mock(Call.class);
-    when(mockcall.execute()).thenReturn(Response.success("testing"));
-    String execute = Retrofit2SyncCall.execute(mockcall);
-    assertEquals("testing", execute);
+  void testExecuteSuccess() throws IOException {
+    Call<String> mockCall = mock(Call.class);
+    String responseBody = "testing";
+    when(mockCall.execute()).thenReturn(Response.success(responseBody));
+    String execute = Retrofit2SyncCall.execute(mockCall);
+    assertThat(execute).isEqualTo(responseBody);
   }
 
   @Test
-  public void testExecuteThrowException() throws IOException {
-    Call<String> mockcall = Mockito.mock(Call.class);
+  void testExecuteThrowException() throws IOException {
+    Call<String> mockCall = mock(Call.class);
     IOException ioException = new IOException("exception test");
-    when(mockcall.execute()).thenThrow(ioException);
-    SpinnakerNetworkException networkEx =
-        assertThrows(
-            SpinnakerNetworkException.class,
-            () -> {
-              Retrofit2SyncCall.execute(mockcall);
-            });
-    assertEquals(ioException, networkEx.getCause());
+    when(mockCall.execute()).thenThrow(ioException);
+
+    HttpUrl url = HttpUrl.parse("http://arbitrary-url");
+    Request mockRequest = mock(Request.class);
+    when(mockCall.request()).thenReturn(mockRequest);
+    when(mockRequest.url()).thenReturn(url);
+
+    SpinnakerNetworkException thrown =
+        catchThrowableOfType(
+            () -> Retrofit2SyncCall.execute(mockCall), SpinnakerNetworkException.class);
+    assertThat(thrown).hasCause(ioException);
+    assertThat(thrown.getUrl()).isEqualTo(url.toString());
   }
 }

kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java

@@ -19,9 +19,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.google.gson.Gson;
 import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
@@ -39,11 +36,11 @@
 import retrofit2.Retrofit;
 import retrofit2.converter.jackson.JacksonConverterFactory;
 
-public class SpinnakerHttpExceptionTest {
+class SpinnakerHttpExceptionTest {
   private static final String CUSTOM_MESSAGE = "custom message";
 
   @Test
-  public void testSpinnakerHttpExceptionFromRetrofitError() {
+  void testSpinnakerHttpExceptionFromRetrofitError() {
     String url = "http://localhost";
     int statusCode = 200;
     String message = "arbitrary message";
@@ -69,7 +66,7 @@ public void testSpinnakerHttpExceptionFromRetrofitError() {
   }
 
   @Test
-  public void testSpinnakerHttpExceptionFromRetrofitException() {
+  void testSpinnakerHttpExceptionFromRetrofitException() {
     final String validJsonResponseBodyString = "{\"name\":\"test\"}";
     ResponseBody responseBody =
         ResponseBody.create(
@@ -86,19 +83,19 @@ public void testSpinnakerHttpExceptionFromRetrofitException() {
     assertThat(retrofit2Service.baseUrl().toString()).isEqualTo(url);
     SpinnakerHttpException notFoundException =
         new SpinnakerHttpException(response, retrofit2Service);
-    assertNotNull(notFoundException.getResponseBody());
+    assertThat(notFoundException.getResponseBody()).isNotNull();
     assertThat(notFoundException.getUrl()).isEqualTo(url);
     assertThat(notFoundException.getReason())
         .isEqualTo("Response.error()"); // set by Response.error
     Map<String, Object> errorResponseBody = notFoundException.getResponseBody();
-    assertEquals(errorResponseBody.get("name"), "test");
-    assertEquals(HttpStatus.NOT_FOUND.value(), notFoundException.getResponseCode());
-    assertTrue(
-        notFoundException.getMessage().contains(String.valueOf(HttpStatus.NOT_FOUND.value())));
+    assertThat(errorResponseBody.get("name")).isEqualTo("test");
+    assertThat(notFoundException.getResponseCode()).isEqualTo(HttpStatus.NOT_FOUND.value());
+    assertThat(notFoundException)
+        .hasMessageContaining(String.valueOf(HttpStatus.NOT_FOUND.value()));
   }
 
   @Test
-  public void testSpinnakerHttpException_NewInstance() {
+  void testSpinnakerHttpException_NewInstance() {
     String url = "http://localhost";
     String reason = "reason";
     Response response = new Response(url, 200, reason, List.of(), null);
@@ -108,10 +105,11 @@ public void testSpinnakerHttpException_NewInstance() {
     } catch (SpinnakerException e) {
       SpinnakerException newException = e.newInstance(CUSTOM_MESSAGE);
 
-      assertTrue(newException instanceof SpinnakerHttpException);
-      assertEquals(CUSTOM_MESSAGE, newException.getMessage());
-      assertEquals(e, newException.getCause());
-      assertEquals(response.getStatus(), ((SpinnakerHttpException) newException).getResponseCode());
+      assertThat(newException).isInstanceOf(SpinnakerHttpException.class);
+      assertThat(newException).hasMessage(CUSTOM_MESSAGE);
+      assertThat(newException).hasCause(e);
+      assertThat(((SpinnakerHttpException) newException).getResponseCode())
+          .isEqualTo(response.getStatus());
       SpinnakerHttpException spinnakerHttpException = (SpinnakerHttpException) newException;
       assertThat(spinnakerHttpException.getUrl()).isEqualTo(url);
       assertThat(spinnakerHttpException.getReason()).isEqualTo(reason);