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());
       } catch (IOException e) {
-        throw new SpinnakerNetworkException(e);
+        throw new SpinnakerNetworkException(e, delegate.request());
       } catch (Exception e) {
-        throw new SpinnakerServerException(e);
+        throw new SpinnakerServerException(e, delegate.request());
       }
       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());
       } else if (t instanceof SpinnakerHttpException) {
         exception = (SpinnakerHttpException) t;
       } else {
-        exception = new SpinnakerServerException(t);
+        exception = new SpinnakerServerException(t, call.request());
       }
       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());
     }
   }
 }

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

@@ -16,14 +16,36 @@
 
 package com.netflix.spinnaker.kork.retrofit.exceptions;
 
+import okhttp3.Request;
+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) {
+  /**
+   * Construct a SpinnakerServerException from retrofit2 with a message and cause (e.g. an exception
+   * converting a response to the specified type).
+   */
+  public SpinnakerConversionException(String message, Throwable cause, Request request) {
+    super(message, cause, request);
+    setRetryable(false);
+  }
+
+  /**
+   * Construct a SpinnakerConversionException from another SpinnakerConversionException (e.g. via
+   * newInstance).
+   */
+  public SpinnakerConversionException(String message, SpinnakerConversionException cause) {
     super(message, cause);
     setRetryable(false);
   }
 
+  /** Construct a SpinnakerConversionException corresponding to a RetrofitError. */
+  public SpinnakerConversionException(RetrofitError e) {
+    super(e);
+    setRetryable(false);
+  }
+
   @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;
 
   /**
@@ -66,6 +64,7 @@ public class SpinnakerHttpException extends SpinnakerServerException {
    */
   private final String reason;
 
+  /** Construct a SpinnakerHttpException corresponding to a RetrofitError. */
   public SpinnakerHttpException(RetrofitError e) {
     super(e);
 
@@ -101,7 +100,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 +111,14 @@ public SpinnakerHttpException(RetrofitError e) {
    */
   public SpinnakerHttpException(
       retrofit2.Response<?> retrofit2Response, retrofit2.Retrofit retrofit) {
+    super(retrofit2Response.raw().request());
     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 +157,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 +194,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 +207,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

@@ -17,19 +17,34 @@
 package com.netflix.spinnaker.kork.retrofit.exceptions;
 
 import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
+import okhttp3.Request;
 import retrofit.RetrofitError;
 
-/** Wraps an exception of kind {@link RetrofitError.Kind} NETWORK. */
+/** Represents a network error while attempting to execute a retrofit http client request. */
 @NonnullByDefault
 public final class SpinnakerNetworkException extends SpinnakerServerException {
-  public SpinnakerNetworkException(Throwable cause) {
-    super(cause);
+
+  /**
+   * Construct a SpinnakerNetworkException from retrofit2 with a cause (e.g. an exception sending a
+   * request or processing a response).
+   */
+  public SpinnakerNetworkException(Throwable cause, Request request) {
+    super(cause, request);
   }
 
-  public SpinnakerNetworkException(String message, Throwable cause) {
+  /**
+   * Construct a SpinnakerNetworkException from another SpinnakerNetworkException (e.g. via
+   * newInstance).
+   */
+  public SpinnakerNetworkException(String message, SpinnakerNetworkException cause) {
     super(message, cause);
   }
 
+  /** Construct a SpinnakerNetworkException corresponding to a RetrofitError. */
+  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,20 +18,57 @@
 
 import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
 import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
+import lombok.Getter;
+import okhttp3.Request;
+import retrofit.RetrofitError;
 
 /** 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);
+  @Getter private final String url;
+
+  /** Construct a SpinnakerServerException corresponding to a RetrofitError. */
+  public SpinnakerServerException(RetrofitError e) {
+    super(e.getMessage(), e.getCause());
+    url = e.getUrl();
   }
 
-  public SpinnakerServerException(Throwable cause) {
+  /**
+   * Construct a SpinnakerServerException from retrofit2 with no cause (e.g. a non-200 http
+   * response).
+   */
+  public SpinnakerServerException(Request request) {
+    super();
+    url = request.url().toString();
+  }
+
+  /**
+   * Construct a SpinnakerServerException from retrofit2 with a cause (e.g. an exception sending a
+   * request or processing a response).
+   */
+  public SpinnakerServerException(Throwable cause, Request request) {
     super(cause);
+    this.url = request.url().toString();
   }
 
-  public SpinnakerServerException() {}
+  /**
+   * Construct a SpinnakerServerException from retrofit2 with a message and cause (e.g. an exception
+   * converting a response to the specified type).
+   */
+  public SpinnakerServerException(String message, Throwable cause, Request request) {
+    super(message, cause);
+    this.url = request.url().toString();
+  }
+
+  /**
+   * Construct a SpinnakerServerException from another SpinnakerServerException (e.g. via
+   * newInstance).
+   */
+  public SpinnakerServerException(String message, SpinnakerServerException cause) {
+    super(message, cause);
+    this.url = cause.getUrl();
+  }
 
   @Override
   public SpinnakerServerException newInstance(String message) {

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());
   }
 }