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.

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

feat(retrofit): add support for retrofit2 clients that handle errors by generating Spinnaker(Server|Http|Network)Exceptions #1004

Merged
merged 16 commits into from
Apr 3, 2023

Conversation

Luthan95
Copy link
Contributor

@Luthan95 Luthan95 commented Jan 9, 2023

Global Error handling approach for changing from retrofit 1.9 to retrofit 2.x version
Creating Custom callAdaptor to handle the failure and throw Custom exception using retrofit2 response
This will be used throw the exceptions SpinnakerHttpException, SpinnakerNetworkException and SpinnakerServerException based on response status. So this will help in handling exception/error globally

References for ErrorHandler:
https://github.com/square/retrofit/blob/parent-2.8.1/samples/src/main/java/com/example/retrofit/ErrorHandlingAdapter.java
https://github.com/square/retrofit/blob/parent-2.8.1/retrofit/src/main/java/retrofit2/DefaultCallAdapterFactory.java

Retrofit2 client with error handler sample:-

Service svc = new Retrofit.Builder()
        .baseUrl("endpoint") // endpoint of service 
        .client(okHttpClient) // okhttp3 client with required interceptors and properties 
        .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())  // Call factory to handle error globally and to make use of custom spinnaker exceptions
        .addConverterFactory(JacksonConverterFactory.create(objectMapper))
        .build()
        .create(Service.class); 

Retrofit2 API sample:

@GET("/fetch/")
 Call<ResponseBody> find(@Query("q") String name);

Use of Retrofit2 client call sample:

By default execute method of retrofit2 call throws IOException, so to handle globally make use of Retrofit2SyncCall.execute(Call call) static method.

ResponseBody findResponse = Retrofit2SyncCall.execute(svc.find("name"))

Note:
New property which added to support the loglevel of retrofit2 client call, default to BASIC level

retrofit2:
  logLevel: BASIC // NONE,HEADERS,BODY

@Luthan95 Luthan95 closed this Jan 9, 2023
@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

  • a37d5c6: added retrofit calladaptor to handle exception globally

  • eb2662e: added retrofit calladaptor to handle exception globally

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@Luthan95 Luthan95 changed the title CVE 2021 0341 retrofit retrofit2 Error handling approch Retrofit2 Error handling: update retrofit 1.9 to 2.x to address CVE-2021-0341 Jan 9, 2023
@Luthan95 Luthan95 changed the title Retrofit2 Error handling: update retrofit 1.9 to 2.x to address CVE-2021-0341 Retrofit2(depedencies): update retrofit 1.9 to 2.x to address CVE-2021-0341 Jan 9, 2023
@Luthan95 Luthan95 reopened this Jan 9, 2023
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch from eb2662e to 054c43b Compare January 9, 2023 10:42
@Luthan95 Luthan95 changed the title Retrofit2(depedencies): update retrofit 1.9 to 2.x to address CVE-2021-0341 fiat(retrofit2): adding custom callbackAdaptor to handle exceptions globally for retrofit2 Jan 12, 2023
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch 3 times, most recently from e33e64d to c56317b Compare January 12, 2023 09:29
@dbyron-sf dbyron-sf marked this pull request as draft January 12, 2023 16:19
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch from 82199c2 to f1d7c93 Compare February 20, 2023 11:02
@Luthan95 Luthan95 changed the title fiat(retrofit2): adding custom callbackAdaptor to handle exceptions globally for retrofit2 refactor(kork): adding custom callbackAdaptor to handle exceptions globally for using retrofit2 Feb 28, 2023
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch from f1d7c93 to 24dba74 Compare February 28, 2023 13:24
@Luthan95 Luthan95 marked this pull request as ready for review February 28, 2023 16:35
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch 4 times, most recently from 21e462f to ed832ce Compare March 6, 2023 03:35
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch from 25fc172 to 142519a Compare April 3, 2023 06:32
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch from 8525955 to 83a6378 Compare April 3, 2023 13:39
@Luthan95 Luthan95 force-pushed the CVE-2021-0341-retrofit-retrofit2 branch from 83a6378 to 7f91fd0 Compare April 3, 2023 14:02
@dbyron-sf dbyron-sf added the ready to merge Approved and ready for merge label Apr 3, 2023
@mergify mergify bot added the auto merged label Apr 3, 2023
@mergify mergify bot merged commit 93ffa14 into spinnaker:master Apr 3, 2023
dbyron-sf added a commit to dbyron-sf/kork that referenced this pull request Apr 3, 2023
now that spinnaker#1004 introduced a HttpLoggingInterceptor.Level bean in kork-web's RetrofitConfiguration class
j-sandy pushed a commit that referenced this pull request Apr 3, 2023
…cy (#1037)

now that #1004 introduced a HttpLoggingInterceptor.Level bean in kork-web's RetrofitConfiguration class
dmart added a commit to dmart/kork that referenced this pull request Nov 6, 2023
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in
spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant,
this results in the headers being added twice to all retrofit1 client requests.
dmart added a commit to dmart/kork that referenced this pull request Nov 13, 2023
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in
spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant,
this results in the headers being added twice to all retrofit1 client requests.
dmart added a commit to dmart/kork that referenced this pull request Nov 13, 2023
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in
spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant,
this results in the headers being added twice to all retrofit1 client requests.
dmart added a commit to dmart/kork that referenced this pull request Nov 13, 2023
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in
spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant,
this results in the headers being added twice to all retrofit1 client requests.
dmart added a commit to dmart/kork that referenced this pull request Nov 13, 2023
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in
spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant,
this results in the headers being added twice to all retrofit1 client requests.
dmart added a commit to dmart/kork that referenced this pull request Nov 13, 2023
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in
spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant,
this results in the headers being added twice to all retrofit1 client requests.
dmart added a commit to dmart/kork that referenced this pull request Nov 13, 2023
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in
spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant,
this results in the headers being added twice to all retrofit1 client requests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged ready to merge Approved and ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants