From a699bb5abce8266fefe2f14a52dc6575f75efe83 Mon Sep 17 00:00:00 2001 From: SheetalAtre Date: Thu, 13 Jul 2023 10:17:40 +0530 Subject: [PATCH 1/2] feat(retrofit): give a custom parsing error message when fails to convert SpinnakerHttpException responsebody --- .../exceptions/RetrofitException.java | 36 +++++++++++++++++-- .../exceptions/SpinnakerHttpException.java | 2 +- .../SpinnakerServerExceptionTest.java | 14 ++++++-- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/RetrofitException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/RetrofitException.java index 29cbab1ea..ed53aa6fe 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/RetrofitException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/RetrofitException.java @@ -18,11 +18,16 @@ import java.io.IOException; import java.lang.annotation.Annotation; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; +import okhttp3.MediaType; import okhttp3.ResponseBody; +import org.springframework.http.HttpStatus; import retrofit2.Converter; import retrofit2.Response; import retrofit2.Retrofit; +import retrofit2.converter.jackson.JacksonConverterFactory; /** * The {@link RetrofitException} class is similar to {@link retrofit.RetrofitError} as RetrofitError @@ -45,6 +50,9 @@ public static RetrofitException httpError(Response response, Retrofit retrofit) */ private final Retrofit retrofit; + private static Map jsonErrorResponseBody = + Map.of("message", "failed to parse response"); + RetrofitException(String message, Response response, Retrofit retrofit) { super(message); @@ -67,16 +75,38 @@ public Response getResponse() { * @throws RuntimeException wrapping the underlying IOException if unable to convert the body to * the specified {@code type}. */ - public T getErrorBodyAs(Class type) { + public Map getErrorBodyAs() { if (response == null) { return null; } - Converter converter = retrofit.responseBodyConverter(type, new Annotation[0]); + Converter converter = + retrofit.responseBodyConverter(Map.class, new Annotation[0]); try { return converter.convert(response.errorBody()); } catch (IOException e) { - throw new RuntimeException(e); + return jsonErrorResponseBody; } } + + static { + getJsonErrorResponseBody(); + } + + private static void getJsonErrorResponseBody() { + ResponseBody jsonErrorResponse = + ResponseBody.create( + MediaType.parse("application/json; charset=utf-8"), + "{\"message\":\"Failed to parse response\"}"); + retrofit2.Response response = + retrofit2.Response.error(HttpStatus.NOT_FOUND.value(), jsonErrorResponse); + Retrofit retrofit2Service = + new Retrofit.Builder() + .baseUrl("http://localhost") + .addConverterFactory(JacksonConverterFactory.create()) + .build(); + + RetrofitException retrofitException = httpError(response, retrofit2Service); + jsonErrorResponseBody = (HashMap) retrofitException.getErrorBodyAs(); + } } diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java index adda01065..62d9e8894 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java @@ -61,7 +61,7 @@ public SpinnakerHttpException(RetrofitException e) { super(e); this.response = null; this.retrofit2Response = e.getResponse(); - responseBody = (Map) e.getErrorBodyAs(HashMap.class); + responseBody = (Map) e.getErrorBodyAs(); this.rawMessage = responseBody != null ? (String) responseBody.getOrDefault("message", e.getMessage()) diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java index eb5386a2a..657393008 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java @@ -102,7 +102,17 @@ public void testSpinnakerHttpExceptionFromRetrofitException() { Map 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()))); + + // Note: Expect custom "Failed to parse response" message, + // instead of a RuntimeException when the converter fails to convert the response body. eg: + // JacksonResponseBodyConverter returns JsonParseException/JsonEOFException which are type of + // RuntimeException. + String expectedMessage = + String.format( + "Status: %s, URL: %s, Message: %s", + HttpStatus.NOT_FOUND.value(), + "http://localhost/", + HttpStatus.NOT_FOUND.value() + " " + "Response.error()"); + assertEquals(expectedMessage, notFoundException.getMessage()); } } From 621f03316c1c09193fc3538c6ff062df6dd6ad15 Mon Sep 17 00:00:00 2001 From: spinnakerbot Date: Tue, 18 Jul 2023 17:35:55 -0400 Subject: [PATCH 2/2] chore(dependencies): Autobump spinnakerGradleVersion (#1080) Co-authored-by: root --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 7b163db85..8aab42b43 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ kotlinVersion=1.4.32 org.gradle.parallel=true -spinnakerGradleVersion=8.30.0 +spinnakerGradleVersion=8.31.0 targetJava11=true includeRuntimes=actuator,core,eureka,retrofit,secrets-aws,secrets-gcp,stackdriver,swagger,tomcat,web