Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(retrofit): add accessor to exception response body as a map #1074

Merged
merged 1 commit into from
Jul 12, 2023
Merged

feat(retrofit): add accessor to exception response body as a map #1074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 16 additions & 25 deletions .../src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@

package com.netflix.spinnaker.kork.retrofit.exceptions;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Preconditions;
import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
import java.util.Optional;
import lombok.Getter;
import java.util.HashMap;
import java.util.Map;
import org.springframework.http.HttpHeaders;
import retrofit.RetrofitError;
import retrofit.client.Response;
Expand All @@ -46,24 +43,29 @@ public class SpinnakerHttpException extends SpinnakerServerException {
*/
private final String rawMessage;

private final Map<String, Object> responseBody;

public SpinnakerHttpException(RetrofitError e) {
super(e);
this.response = e.getResponse();
this.retrofit2Response = null;
RetrofitErrorResponseBody body =
(RetrofitErrorResponseBody) e.getBodyAs(RetrofitErrorResponseBody.class);
responseBody = (Map<String, Object>) e.getBodyAs(HashMap.class);

this.rawMessage =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling we'll end up initializing rawMessage something like this

this.rawMessage = e.getMessage();

when we start handling e.g. non-json response bodies. It's possible that would simplify the code here even before that. I'm OK to leave this as it is for now....especially if there's a way to make rawMessage totally disappear.

Optional.ofNullable(body).map(RetrofitErrorResponseBody::getMessage).orElse(e.getMessage());
responseBody != null
? (String) responseBody.getOrDefault("message", e.getMessage())
: e.getMessage();
}

public SpinnakerHttpException(RetrofitException e) {
super(e);
this.response = null;
this.retrofit2Response = e.getResponse();
RetrofitErrorResponseBody body =
(RetrofitErrorResponseBody) e.getErrorBodyAs(RetrofitErrorResponseBody.class);
responseBody = (Map<String, Object>) e.getErrorBodyAs(HashMap.class);
this.rawMessage =
Optional.ofNullable(body).map(RetrofitErrorResponseBody::getMessage).orElse(e.getMessage());
responseBody != null
? (String) responseBody.getOrDefault("message", e.getMessage())
: e.getMessage();
}

private final String getRawMessage() {
Expand Down Expand Up @@ -95,6 +97,7 @@ public SpinnakerHttpException(String message, SpinnakerHttpException cause) {
this.response = cause.response;
this.retrofit2Response = cause.retrofit2Response;
rawMessage = null;
this.responseBody = cause.responseBody;
}

public int getResponseCode() {
Expand Down Expand Up @@ -151,19 +154,7 @@ public SpinnakerHttpException newInstance(String message) {
return new SpinnakerHttpException(message, this);
}

@Getter
// Use JsonIgnoreProperties because some responses contain properties that
// cannot be mapped to the RetrofitErrorResponseBody class. If the default
// JacksonConverter (with no extra configurations) is used to deserialize the
// response body and properties other than "message" exist in the JSON
// response, there will be an UnrecognizedPropertyException.
@JsonIgnoreProperties(ignoreUnknown = true)
private static final class RetrofitErrorResponseBody {
private final String message;

@JsonCreator
RetrofitErrorResponseBody(@JsonProperty("message") String message) {
this.message = message;
}
public Map<String, Object> getResponseBody() {
dbyron-sf marked this conversation as resolved.
Show resolved Hide resolved
return this.responseBody;
}
}
5 changes: 5 additions & 0 deletions ...est/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
package com.netflix.spinnaker.kork.retrofit.exceptions;

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.netflix.spinnaker.kork.exceptions.SpinnakerException;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import okhttp3.MediaType;
import okhttp3.ResponseBody;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -96,6 +98,9 @@ public void testSpinnakerHttpExceptionFromRetrofitException() {
.build();
RetrofitException retrofitException = RetrofitException.httpError(response, retrofit2Service);
SpinnakerHttpException notFoundException = new SpinnakerHttpException(retrofitException);
assertNotNull(notFoundException.getResponseBody());
dbyron-sf marked this conversation as resolved.
Show resolved Hide resolved
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())));
Expand Down