-
Notifications
You must be signed in to change notification settings - Fork 172
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.
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
refactor(retrofit/test): simplify the code by replacing RetrofitException.httpError with SpinnakerHttpException #1079
refactor(retrofit/test): simplify the code by replacing RetrofitException.httpError with SpinnakerHttpException #1079
Conversation
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
this.retrofit2Response = syncResp; | ||
this.response = null; | ||
this.retrofit = retrofit; | ||
responseBody = this.getErrorBodyAs(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may belong in a followup PR, but...if we can't parse the response body as json to build a map, we'd eventually like the exception message to include some part of the response. I think that means we need a function like processResponse that sets responseBody and either rawMessage or the "plain" message field in this exceptional case.
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
00d7d58
to
63ffe23
Compare
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
211f0c9
to
d189f3d
Compare
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
...fit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
Outdated
Show resolved
Hide resolved
d189f3d
to
95e8075
Compare
00fc79f
to
b4d1b59
Compare
3ae55e5
to
a2a0b9d
Compare
a2a0b9d
to
ba4c5e9
Compare
befd121
to
ba4c5e9
Compare
8e9f599
to
ba4c5e9
Compare
a76d050
to
ba4c5e9
Compare
bfc2b63
to
d5a91bf
Compare
…from a RetrofitError
…to allow to set custom properties in error response body
…o remove RetrofitException class later
… it will always be of type Map
…n to remove createSpinnakerHttpException() later
…etrofitException.httpError() later
…in all files to simplify code
d5a91bf
to
1c45e6f
Compare
and pass it a Retrofit object so we no longer need a Retrofit member
… in SpinnakerHttpException since most members and methods are nullable
…ion.getRawMessage declaration
refactor(retrofit/test): simplify the code by replacing RetrofitException.httpError with SpinnakerHttpException
and removing RetrofitException