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

Retrofit 3.x Things #2180

Open
JakeWharton opened this issue Jan 30, 2017 · 55 comments
Open

Retrofit 3.x Things #2180

JakeWharton opened this issue Jan 30, 2017 · 55 comments
Milestone

Comments

@JakeWharton
Copy link
Collaborator

JakeWharton commented Jan 30, 2017

Currently no plans for a Retrofit 3.x. Just tracking changes in a single issue instead of many.

  • Mock behavior should invoke the implementation method for each Call.execute() / Call.enqueue(). Currently it only invokes it once and reuses the result for every cloned Call.
  • Eliminate Retrofit-wide knowledge of callback executor. This should be at best a first-party CallAdapter.Factory which you can apply (if you want) and which can target all calls or just those annotated with something like @MainThread.
@JakeWharton JakeWharton added this to the 3.0 milestone Jan 30, 2017
@TobiasReich
Copy link

Will there be a simple way in order to retry a call n times?
This would come very handy when there is a call that should be retried after an error.
I was hoping for something like: request.retry(5);

@naturalwarren
Copy link
Contributor

To do this with Observables we use a custom transformer that gets tacked onto request invocations that takes in the number of retries.

@Jawnnypoo

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@Jawnnypoo

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@JakeWharton
Copy link
Collaborator Author

@TobiasReich no plans for retry. You can already call clone() on a Call to retry and so implementing that at the application level seems more appropriate. You can write a CallAdapter to return a custom type that provides a nice API for retry. Because of the behavior in sync vs. async mode and what executor the delay happens on being unclear I don't think I want it in the library.

@ylfzq
Copy link

ylfzq commented Feb 28, 2017

You can add code below to your okhttp interceptor to archieve retry mechanism.

new OkHttpClient.Builder()
  .addInterceptor(new RetryInterceptor(3)) // must add at first
  // ...
  .build();

The RetryInterceptor is

public class RetryInterceptor implements Interceptor {
  private final int mRetryTimes;

  public RetryInterceptor(int retryTimes) {
    mRetryTimes = retryTimes;
  }

  @Override
  public Response intercept(Chain chain) throws IOException {
    int retryTimesLeft = mRetryTimes;
    for (; retryTimesLeft > 0; --retryTimesLeft) {
      try {
        return chain.proceed(chain.request());
      } catch (IOException ignore) {
      }
    }
    throw new IOException("still fail after retry " + mRetryTimes + " times");
  }
}

The code is not tested yet. But I think it's quite simple to understand.

@Skullper

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@msangel
Copy link

msangel commented Aug 12, 2017

Return easy error api from Retrofit1.

@jaredsburrows

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@jaredsburrows

This comment has been minimized.

@msangel

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@msangel
Copy link

msangel commented Aug 28, 2017

Most of people get http error with adequate, well-formatted message. Yep, not all. But still, most cases the response can be converted well with built-in or custom converters, like those for 2xx responces. And this works perfect in version1. People need this. On stackoverflow I saw workarounds like creating new retrofit instance(or getting instance as singletons) just for accessing converters. Or others solutions(http://bytes.babbel.com/en/articles/2016-03-16-retrofit2-rxjava-error-handling.html)

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Aug 28, 2017 via email

@jaredsburrows
Copy link
Contributor

@JakeWharton
Copy link
Collaborator Author

@msangel
Copy link

msangel commented Aug 29, 2017

Yep. The first sample is good. It is worst then retrofit1 solution just in that moment that error know nothing about it's context. Adding to the error (transient) link to the retrofit instance it was created with will solve the gap. And maybe adding method like getErrorBodyAs(SomeType.class) to the HttpError will fulfill it completely.

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Aug 29, 2017 via email

@vidhithakrar
Copy link

Any plan to support custom annotation on an interface in 3.x?

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Sep 7, 2017 via email

@technoir42
Copy link
Contributor

technoir42 commented Sep 7, 2017

Perhaps something like:

@BaseUrl("https://host1.mydomain.com")
public interface MyService1 {
    @GET("get_something")
    Call<Something> getSomething();
    // ... Many other methods that use the same baseUrl
}

Where @BaseUrl is a user-defined annotation. Then CallAdapter.Factory should receive not just method annotations but also the annotations declared on the interface.
I can imagine why this could be useful. Instead of creating a customized Retrofit client before calling retrofit.create(X.class) one could create a custom CallAdapter.Factory that would parse those annotations and provide an accordingly customized Retrofit instance to a CallAdapter.

@vidhithakrar
Copy link

vidhithakrar commented Sep 7, 2017

@zergtmn Yes, something like that. One more thing to add is we get Method level annotation in the CallAdapterFactory but not the Method Parameter level.

public interface FooServices {
    @GET("/path")
    @MethodCustom
    Call<Foo> foo(@Custom String custom);
}

In the CallAdapterFactory, We get @MethodCustom annotation but not the @Custom.

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Sep 7, 2017 via email

@janheinrichmerker
Copy link

One major point on my whish-list for Retrofit 3 is better type converters.
Moshi does this excellently and Google's Room is by far the most intuitive solution.
I know that Retrofit is a bit different in that it features different adapters for the request body and for strings, but it feels like the converter API needs some improvement.
Think of use cases where you want to have one type converter convert from ResponseBody to Foo and another that can then convert from Foo to Bar. That way you could e.g. parse.
I've personally worked on a conversion API some time ago and would be happy to help in the development of Retrofit 3.

@msangel
Copy link

msangel commented Oct 23, 2017

Custom annotation example: interface with the filename as a parameter. We are registering a custom handler for methods with such annotations. Then when the method gets called - the execution will be delivered to the handler. So I will read the file, will process it(compress in my case) and will send in the custom manner.
And I do not need to write any custom wrapper around the interface just for inject some specific functionality to the generated class.
Another case: special custom AUTH, when the credentials are just additional request parameters(quite popular case), so we will write a custom handler that will set the credentials. So setting custom annotation and writing handler for it will solve the problem.
Another case: we can write one custom annotation for queries to one host, and the second - for queries to the second host.
I believe this annotation should be able to change not only request but the response as well. Actually, you have already builtin logic for convert the response via converters, its good, but still few things there are missing (current retrofit object);

@D330
Copy link

D330 commented Nov 11, 2017

Tag in request

@Bmooij

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@derekm
Copy link

derekm commented Dec 21, 2017

It would be nice if Retrofit entirely adopted JAX-RS for its interface definitions in 3.0. Perhaps a compat jar could be made that keeps the proprietary annotations working as Retrofit moves on to use the standard annotations.

MicroProfile Rest Client 1.0 has entered released candidate status, and it is another JAX-RS-based proxy client. The other JAX-RS-based proxy client I've used extensively is Jersey Proxy Client (WebResourceFactory).

I personally much prefer the JAX-RS approach over Retrofit's proprietary annotations. I like to reuse my interface definitions on both the client and the server sides.

@janheinrichmerker
Copy link

I like the idea of implementing the JSR-RS annotations!
If you use exactly the same interface definition and standard on both the server and client side there's not much room for bugs and errors anymore.
And it shouldn't be too hard either as these annotations work very similar to those from Retrofit.

@nealsanche

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@johnrengelman
Copy link

@JakeWharton I've also recently found a use case for custom annotations in the retrofit module for Ratpack. The goal is to allow specifying customer parameters for each individual request type, such as connect/read timeout, max body size, etc.

Additionally, I've found myself limited in trying to implement this, because I can only get the annotations in the CallAdapterFactory but the actual binding to the Ratpack HttpClient happens in the CallFactory and these 2 factories use different types. CallAdapaterFactory deals with retrofit2.Call, where as CallFactory deals with okhttp3.Call.
This is an annoying inconsistency in the API.

I know Retrofit has been designed around OkHttp for a while, but with a few changes it could be decoupled entirely from the backing http client implementation.

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Jan 8, 2018 via email

@johnrengelman
Copy link

@JakeWharton that would be great for my use case. Would love to see that in a 2.x release :)

@Edward608
Copy link

Could there be a @Throw(CustomException) for individual endpoint?

In my case I want to have a unique exception for each api indicating their failure instead of general HttpException, but for now I can only use the work around by apiCall.onErrorResumeNext { emit(CustomException) }

Its just my personal opinion, would love to see your feedback about this.

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Feb 1, 2018 via email

@Edward608
Copy link

@JakeWharton Could you give me a little bit more hint about this? I see the Annotation[] is being passed to the CallAdapterFactory, so this should be the only place to read those annotations?

@maher640
Copy link

maher640 commented Feb 26, 2018

how about executing the same request x times in one go and waiting for the data from all requests. Example: getUser(1), getUser(2), getUser(3), ... etc

@KangoV
Copy link

KangoV commented Feb 26, 2018

Regarding JAX-RS annotations, Retrofit could handle them how Guice handles javax.inject. It could look for them on the classpath/modulepath and if not there, will not even look for them. We use Ratpack and Retrofit together for our services (no JEE in sight) and would be a shame to have to include a JAX-RS dependency.

@derekm
Copy link

derekm commented Mar 30, 2018

JAXRS devs are starting discussions on JAXRS 2.2. The 2.2 spec will likely include proxy client details that will set about obsoleting Retrofit.

See, for example:
https://dev.eclipse.org/mhonarc/lists/jaxrs-dev/msg00261.html
... and ...
jakartaee/rest#56
... and ...
jakartaee/rest#598

Retrofit (& Moshi) would do well to align themselves with these emerging standards rather than remaining in an incompatible, proprietary niche API space.

@technoir42
Copy link
Contributor

technoir42 commented Mar 30, 2018

@derekm if Retrofit adopted JAX-RS how would it work on Android? There is no javax.ws.rs on Android.

@derekm
Copy link

derekm commented Mar 30, 2018

Retrofit wouldn't need to implement the server-side of JAX-RS. Retrofit 3 could easily be a client-side only implementation of JAX-RS (e.g., https://docs.oracle.com/javaee/7/api/javax/ws/rs/client/package-summary.html + whatever is come up with in JAX-RS 2.2's Retrofit/MP-Rest-Client/WebResourceFactory-alike proxy client), with optional jars for backcompat with previous Retrofit releases.

With MP Rest Client 1.0 & JAX-RS 2.2 standardizing proxy clients, what you will start to see is server-side projects offering up JAX-RS/JSON-P/JSON-B/JAXB interface jars for client-side consumption.

Retrofit should make itself compatible with these emerging no-code & code-light interfaces & clients.

@hpanahiird
Copy link

please add a way to retry a failed callback with different base url.

@juliocbcotta
Copy link
Contributor

Can we be allowed to change the Coroutines dispatcher? It would help a lot with idling resources in UI Tests.

@JakeWharton
Copy link
Collaborator Author

No dispatcher is used in our coroutine integration.

@juliocbcotta
Copy link
Contributor

No dispatcher is used in our coroutine integration.

Ok, Can we have a way to set the Dispatcher that we want to use when making a request/parsing? So we could centralize this configuration in one place and not at every request and swap the Dispatcher for one that Espresso waits to be idle.

@JakeWharton
Copy link
Collaborator Author

The threading is controlled by OkHttp. Retrofit does not have any control over it.

@juliocbcotta
Copy link
Contributor

So, it means that the parsing is done in the calling thread and not in a background thread? Would this be something that needs to be asked in the parsing libs repository or could it be supported by retrofit 3.0?

@JakeWharton
Copy link
Collaborator Author

It is done on the background thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests