feat(retrofit): Make HTTP method available in SpinnakerServerException (

spinnaker#1149) * feat(retrofit): added httpMethod variable and getter to SpinnakerServerException, updated tests * feat(retrofit): added assertions to show the http method variable in a spinnaker exception will be null if created from RetrofitError --------- Co-authored-by: abe garcia <abraham_garcia-ortiz1@homedepot.com>
dbyron-sf · Feb 19, 2024 · ef5d317 · ef5d317
1 parent a24bcbe
commit ef5d317
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 0 deletions.
diff --git a/...rc/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerException.java b/...rc/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerException.java
@@ -27,11 +27,13 @@
 public class SpinnakerServerException extends SpinnakerException {
 
   @Getter private final String url;
+  @Getter private final String httpMethod;
 
   /** Construct a SpinnakerServerException corresponding to a RetrofitError. */
   public SpinnakerServerException(RetrofitError e) {
     super(e.getMessage(), e.getCause());
     url = e.getUrl();
+    httpMethod = null;
   }
 
   /**
@@ -41,6 +43,7 @@ public SpinnakerServerException(RetrofitError e) {
   public SpinnakerServerException(Request request) {
     super();
     url = request.url().toString();
+    httpMethod = request.method();
   }
 
   /**
@@ -50,6 +53,7 @@ public SpinnakerServerException(Request request) {
   public SpinnakerServerException(Throwable cause, Request request) {
     super(cause);
     this.url = request.url().toString();
+    this.httpMethod = request.method();
   }
 
   /**
@@ -59,6 +63,7 @@ public SpinnakerServerException(Throwable cause, Request request) {
   public SpinnakerServerException(String message, Throwable cause, Request request) {
     super(message, cause);
     this.url = request.url().toString();
+    this.httpMethod = request.method();
   }
 
   /**
@@ -68,6 +73,7 @@ public SpinnakerServerException(String message, Throwable cause, Request request
   public SpinnakerServerException(String message, SpinnakerServerException cause) {
     super(message, cause);
     this.url = cause.getUrl();
+    this.httpMethod = cause.getHttpMethod();
   }
 
   @Override

diff --git a/.../test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java b/.../test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java
@@ -28,6 +28,7 @@
 import okhttp3.MediaType;
 import okhttp3.ResponseBody;
 import org.junit.jupiter.api.Test;
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import retrofit.RetrofitError;
 import retrofit.client.Response;
@@ -63,6 +64,7 @@ void testSpinnakerHttpExceptionFromRetrofitError() {
         .isEqualTo("Status: " + statusCode + ", URL: " + url + ", Message: " + message);
     assertThat(spinnakerHttpException.getUrl()).isEqualTo(url);
     assertThat(spinnakerHttpException.getReason()).isEqualTo(reason);
+    assertThat(spinnakerHttpException.getHttpMethod()).isNull();
   }
 
   @Test
@@ -92,6 +94,7 @@ void testSpinnakerHttpExceptionFromRetrofitException() {
     assertThat(notFoundException.getResponseCode()).isEqualTo(HttpStatus.NOT_FOUND.value());
     assertThat(notFoundException)
         .hasMessageContaining(String.valueOf(HttpStatus.NOT_FOUND.value()));
+    assertThat(notFoundException.getHttpMethod()).isEqualTo(HttpMethod.GET.toString());
   }
 
   @Test

diff --git a/...ava/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java b/...ava/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java
@@ -31,6 +31,7 @@
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import retrofit2.Call;
 import retrofit2.Retrofit;
@@ -145,6 +146,21 @@ void testResponseHeadersInException() {
         .isEqualTo(mockWebServer.url("/retrofit2").toString());
   }
 
+  @Test
+  void testHttpMethodInException() {
+    // Check http request method is retrievable from a SpinnakerHttpException
+    mockWebServer.enqueue(
+        new MockResponse()
+            .setResponseCode(HttpStatus.BAD_REQUEST.value())
+            .setBody(responseBodyString));
+    SpinnakerHttpException spinnakerHttpException =
+        catchThrowableOfType(
+            () -> retrofit2Service.deleteRetrofit2().execute(), SpinnakerHttpException.class);
+    assertThat(spinnakerHttpException.getUrl())
+        .isEqualTo(mockWebServer.url("/retrofit2").toString());
+    assertThat(spinnakerHttpException.getHttpMethod()).isEqualTo(HttpMethod.DELETE.toString());
+  }
+
   @Test
   void testNotParameterizedException() {
     IllegalArgumentException illegalArgumentException =
@@ -177,6 +193,9 @@ interface Retrofit2Service {
 
     @retrofit2.http.GET("/retrofit2/wrongReturnType")
     DummyWithExecute testWrongReturnType();
+
+    @retrofit2.http.DELETE("/retrofit2")
+    Call<String> deleteRetrofit2();
   }
 
   @Test

diff --git a/...ava/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandlerTest.java b/...ava/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofitErrorHandlerTest.java
@@ -176,6 +176,17 @@ void testResponseHeadersInException() {
     assertThat(spinnakerHttpException.getUrl()).isEqualTo(mockWebServer.url("/foo").toString());
   }
 
+  @Test
+  void testExceptionFromRetrofitErrorHasNullHttpMethod() {
+    mockWebServer.enqueue(
+        new MockResponse()
+            .setResponseCode(HttpStatus.BAD_REQUEST.value())
+            .setHeader("Test", "true"));
+    SpinnakerHttpException spinnakerHttpException =
+        catchThrowableOfType(() -> retrofitService.getFoo(), SpinnakerHttpException.class);
+    assertThat(spinnakerHttpException.getHttpMethod()).isNull();
+  }
+
   @Test
   void testSimpleSpinnakerNetworkException() {
     String message = "my custom message";

diff --git a/...est/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java b/...est/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java
@@ -24,6 +24,7 @@
 import java.util.List;
 import okhttp3.Request;
 import org.junit.jupiter.api.Test;
+import org.springframework.http.HttpMethod;
 import retrofit.RetrofitError;
 import retrofit.client.Response;
 import retrofit.converter.ConversionException;
@@ -41,6 +42,19 @@ void testSpinnakerNetworkExceptionWithUrl() {
     SpinnakerNetworkException spinnakerNetworkException =
         new SpinnakerNetworkException(cause, request);
     assertThat(spinnakerNetworkException.getUrl()).isEqualTo(url);
+    assertThat(spinnakerNetworkException.getHttpMethod()).isEqualTo(HttpMethod.GET.toString());
+  }
+
+  @Test
+  void testSpinnakerNetworkExceptionWithSpecificMethod() {
+    Throwable cause = new Throwable("arbitrary message");
+    String url = "http://some-url/";
+    Request request =
+        new Request.Builder().url(url).method(HttpMethod.DELETE.toString(), null).build();
+    SpinnakerNetworkException spinnakerNetworkException =
+        new SpinnakerNetworkException(cause, request);
+    assertThat(spinnakerNetworkException.getUrl()).isEqualTo(url);
+    assertThat(spinnakerNetworkException.getHttpMethod()).isEqualTo(HttpMethod.DELETE.toString());
   }
 
   @Test
@@ -70,6 +84,19 @@ void testSpinnakerServerExceptionWithUrl() {
     SpinnakerServerException spinnakerServerException =
         new SpinnakerServerException(cause, request);
     assertThat(spinnakerServerException.getUrl()).isEqualTo(url);
+    assertThat(spinnakerServerException.getHttpMethod()).isEqualTo(HttpMethod.GET.toString());
+  }
+
+  @Test
+  void testSpinnakerServerExceptionWithSpecificMethod() {
+    Throwable cause = new Throwable("arbitrary message");
+    String url = "http://some-url/";
+    Request request =
+        new Request.Builder().url(url).method(HttpMethod.DELETE.toString(), null).build();
+    SpinnakerServerException spinnakerServerException =
+        new SpinnakerServerException(cause, request);
+    assertThat(spinnakerServerException.getUrl()).isEqualTo(url);
+    assertThat(spinnakerServerException.getHttpMethod()).isEqualTo(HttpMethod.DELETE.toString());
   }
 
   @Test