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 2.0 API Spec (Working Draft) #297

Closed
JakeWharton opened this Issue Aug 8, 2013 · 106 comments

Comments

@JakeWharton
Copy link
Collaborator

JakeWharton commented Aug 8, 2013

This is a living spec for what will be v2 of Retrofit.

Goals

  • Broaden the scope of how you can process successful responses and handle the results of various exceptions throughout the stack. Currently there's a mix of exceptions, wrapped types, and callbacks depending on the usage of synchronous or asynchronous API declarations.
  • Unify API declaration so that both synchronous and asynchronous consumption of a resource does not change its declaration.
  • Introduce better request and response interceptors which allow for both tweaks to existing requests or full on replacement (e.g., for signing purposes).
  • Allow simple request retry and cancel support without the need to talk to the RestAdapter or generated interface instance.
  • Completely decouple interface declaration parsing from request invocation and processing and expose the shim layer between the two as a extension-based system for customization. This will be used to (hopefully) facilitate RxJava integration as an extensions rather than being baked into the core of the library itself

Changes

Annotation Processors

An annotation processor will be embedded inside the core artifact for compile-time verification of REST API interface declarations.

A second annotation processor will be provided as a separate artifact for full code generation of a class implementation of each API interface.

The RestAdapter will always do a read-through cached lookup for the generated classes since it has no knowledge of whether the code-gen processor was used and we don't want to place the burden on the caller either.

Request Object

All interface declarations will be required to return an object through which all interaction will occur. The behavior of this object will be similar to a Future and will be generic typed (T) for the success response type (ref: #231).

@GET("/foo")
Call<Foo> getFoo();

Callers can synchronously call .execute() which will return type T. Exceptions will be thrown for any error, network error, unsuccessful response, or unsuccessful deserialization of the response body. While these exceptions will likely extend from the same supertype, it's unclear as to whether that supertype should be checked or unchecked.

interface Api {
  @GET("/{user}/tweets")
  Call<List<Tweet>> listTweets(@Path("user") String username);
}
List<Tweet> tweets = api.listTweets("JakeWharton").execute();

Callers can also supply callbacks to this object for asynchronous notification of the response. The traditional Callback<T> of the current version of Retrofit will be available. One change will be that the error object passed to failure will not be the same exception as would be thrown in synchronous execution but rather something a bit more transparent to the underlying cause.

interface Api {
  @GET("/{user}/tweets")
  Call<List<Tweet>> listTweets(@Path("user") String username);
}
Call<List<Tweet>> c = api.listTweets("JakeWharton");
c.execute(new Callback<List<Tweet>>() {
  @Override public void success(List<Tweet> response) { }
  // ...
}

TODO describe error handling

public abstract class ResponseCallback<T> {
  public abstract void success(T response);

  public void networkError(IOException e) {
    error(ErrorType.NETWORK, -1, e);
  }
  public void processingError(Exception e) {
    error(ErrorType.PROCESSING, -1, e);
  }
  public void httpError(int status, ...) {
    error(ErrorType.HTTP, status, e);
  }
  public void error(ErrorType errorType, int status, Exception e) {
    throw new RuntimeException(.., e);
  }

  public enum ErrorType { NETWORK, PROCESSING, HTTP }
}

The call object is also an obvious place for handling the retry and cancelation of requests. Both are a simple, no-args method on the object which can only be called at appropriate times.

  • cancel() is a no-op after the response has been received. In all other cases the method will set any callbacks to null (thus freeing strong references to the enclosing class if declared anonymously) and render the request object dead. All future interactions with the request object will throw an exception. If the request is waiting in the executor its Future will be cancelled so that it is never invoked.
  • retry() will re-submit the request onto the backing executor without passing through any of the mutating pipeline described above. Retrying a request is only available after a network error or 5XX response. Attempting to retry a request that is currently in flight, after a non-5XX response, after an unexpected error, or after calling cancel() will throw an exception.

Extension System

In order to facilitate libraries which offer semantics on both synchronous and asynchronous operations without requiring a wrapper, we will introduce a system which we will tentatively refer to as extensions. An extension instance will be registered with the RestAdapter and associate itself as handling an explicit interface method return type.

By default, Retrofit will install its own extension which handles the aforementioned Call type for both synchronous and asynchronous request execution.

The current (albeit experimentally denoted) RxJava integration will also be provided as an opt-in extension. This extension will register itself as handling the Observable type which will allow the declaration of interfaces which return this type.

interface FooService {
  @GET("/foos")
  Observable<Foo> getFoo();
}

And more...

TODO!

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Aug 8, 2013

@adriancole

This comment has been minimized.

Copy link
Contributor

adriancole commented Aug 8, 2013

@adriancole

This comment has been minimized.

Copy link
Contributor

adriancole commented Aug 8, 2013

guys. this is great. looking forward to helping

@pforhan

This comment has been minimized.

Copy link
Contributor

pforhan commented Aug 8, 2013

Looks awesome. Love that Callbacks now become part of the real api, at compile-time.

@JakeWharton JakeWharton referenced this issue Aug 8, 2013

Closed

Patch 1 #299

@austynmahoney

This comment has been minimized.

Copy link
Contributor

austynmahoney commented Aug 9, 2013

Any examples of what error handling may look like in 2.0?

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Aug 9, 2013

@austynmahoney I'm tempted to revert a bit back to pre-1.0 days with multiple callback methods. Something like this:

public interface Callback<T> {
  void success(T result); // 2XX responses
  void networkError(IOException e); // IOException from underlying input stream
  void unexpectedError(Exception e); // Error from converter or internal Retrofit error.
  void httpError(int status, ...); // non-2XX responses
}

What do you think?

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Aug 9, 2013

Or maybe!

public interface Callback<T> {
  void success(T result); // 2XX responses
}
public interface ErrorCallback {
  void networkError(IOException e); // IOException from underlying input stream
  void unexpectedError(Exception e); // Error from converter or internal Retrofit error.
  void httpError(int status, ...); // non-2XX responses

  class EmptyErrorCallback implements ErrorCallback {
    @Override public void networkError(IOException e) {}
    @Override public void unexpectedError(Exception e) {}
    @Override public void httpError(int status, ...) {}
  }
}
@berwyn

This comment has been minimized.

Copy link

berwyn commented Aug 9, 2013

Just from what I see here, I'd say the second form seems preferable to me. A bit more granular control over errors vs success, and allows independent re-use of callbacks as needed.

@andreaturli

This comment has been minimized.

Copy link

andreaturli commented Aug 9, 2013

I'd be really interested in seeing plans related to the support for different authentication mechanisms (i.e. oauth). Any thoughts on that?

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Aug 9, 2013

Presumably the request and response interceptor/replacement would be enough to enable that. I have no plans to work on anything around oauth, digest, or the like myself. It will have to be contributed during the 2.0 development.

@emilsjolander

This comment has been minimized.

Copy link

emilsjolander commented Aug 9, 2013

I would like to suggest that the Future class has one single method to send the request, this being the get() method. So instead of having a get() method for synchronous operations and an addCallback() (which as i understand it would send the request much like get()) method for asynchronous ones i suggest having multiple overloaded get() methods.

Synchronous request:

Response <Tweet> r = api.listTweets("JakeWharton");
List<Tweet> tweets = r.get();

Asynchronous request:

Response <Tweet> r = api.listTweets("JakeWharton");
r.get(new Callback<List<Tweet>>() {
  @Override public void onSuccess(List<Tweet> response) { }
});

Asynchronous request (Observable):

Response <Tweet> r = api.listTweets("JakeWharton");
r.get(new ObservableCallback<Tweet>() {
  @Override public void onNext(Tweet tweet) { }
  // ...
});

I think this way of doing it would make it clearer what is happening for someone not familiar with the library. It also makes it clearer that you are done with the Future instance.

Regarding the error handling i very much like what @JakeWharton suggested in his latest comment. I would however make a suggestion to remove the EmptyErrorCallback base implementation. This encourages people to write more error prone code by not requiring handling of all the errors. If the user of retrofit would like to then that person can easily make a base implementation like EmptyErrorCallback is their own codebase.

@austynmahoney

This comment has been minimized.

Copy link
Contributor

austynmahoney commented Aug 9, 2013

I like the new ErrorCallback paradigm, it will be much easier for developers to know what actually happened and have less complex error handlers.

One thing I am worried about though is what the code will look like when you try to figure out the difference between a converter and an internal exception. Does anyone handle these two types of errors differently? Is there a recovery path that someone would want to go down if it was a converter error? I can't quite decide if this is important enough to separate converter and internal error into two different methods in the interface. Input on this?

I agree with @emilsjolander. The EmptyErrorCallback will encourage devs to ignore errors completely. There isn't much extra work if they really want to do it themselves.

Just to make sure, 3xx responses are not getting passed into httpError() right?
void httpError(int status, ...); // non-2XX responses

@austynmahoney

This comment has been minimized.

Copy link
Contributor

austynmahoney commented Aug 9, 2013

Another thing I'd like to see in 2.0 is generic error handling. Right now the handler you can add to RestAdapter.Builder seems to only be called when an internal error occurs. I'd like to be able to give the builder an ErrorCallback implementation that handles a subset of errors and propagates the ones it does not. It could possibly work like the touch event callback system on views.

class GenericErrorCallback implements ErrorCallback {
    @Override public void networkError(IOException e) {...}
    @Override public void unexpectedError(Exception e) {...}
    @Override public void httpError(int status, ...) {
         if(status == HTTP_401) {
              // Do something
         }
         // We need something here like a boolean or return type so the error can be propagated to the ErrorCallback on the actual request.
     }
  }
@adriancole

This comment has been minimized.

Copy link
Contributor

adriancole commented Aug 9, 2013

I think ObservableCallback should be named Observer and the method that
accepts it called subscribe and return a Subscription with a single method
unsubscribe. This is nice and how rx does it.

Observer would have an onError(Throwable).

I'm not sure about the mutli ErrorCallback yet and would need to play with
it.

I wonder if we could do a working design that could be published from a
branch to sonatype snapshots repo. This could be an intentional throwaway
in a different namespace. I wouldn't mind making 2 versions of my clients
until we figure it out.

On Friday, August 9, 2013, Emil Sjölander wrote:

I would like to suggest that the Future class has one single method to
send the request, this being the get() method. So instead of having a
get() method for synchronous operations and an addCallback() (which as i
understand it would send the request much like get()) method for
asynchronous ones i suggest having multiple overloaded get() methods.

Synchronous request:

Response r = api.listTweets("JakeWharton");List tweets = r.get();

Asynchronous request:

Response r = api.listTweets("JakeWharton");r.get(new Callback<List>() {
@override public void onSuccess(List response) { }});

Asynchronous request (Observable):

Response r = api.listTweets("JakeWharton");r.get(new ObservableCallback() {
@override public void onNext(Tweet tweet) { }
// ...});

I think this way of doing it would make it clearer what is happening for
someone not familiar with the library. It also makes it clearer that you
are done with the Future instance.

Regarding the error handling i very much like what @JakeWhartonhttps://github.com/JakeWhartonsuggested in his latest comment. I would however make a suggestion to
remove the EmptyErrorCallback base implementation. This encourages people
to write more error prone code by not requiring handling of all the errors.
If the user of retrofit would like to then that person can easily make a
base implementation like EmptyErrorCallback is their own codebase.


Reply to this email directly or view it on GitHubhttps://github.com//issues/297#issuecomment-22379156
.

@austynmahoney

This comment has been minimized.

Copy link
Contributor

austynmahoney commented Aug 9, 2013

A 2.0 alpha release would be nice to play around with. This way we see how it feels using the new API in a real app.

@austynmahoney

This comment has been minimized.

Copy link
Contributor

austynmahoney commented Aug 9, 2013

Just hit a spot in an app I am working on where a 204 No Content response is returned. A Void response in the callback would be useful here. How will this be handled in v2? The Callback<Void> method suggested in #287 looks clean if you leave out the VoidResponse stuff.

@adriancole

This comment has been minimized.

Copy link
Contributor

adriancole commented Aug 10, 2013

Absolutely agree callback void needs to be supported for this case.

On Friday, August 9, 2013, Austyn Mahoney wrote:

Just hit a spot in an app I am working on where a 204 No Content response
is returned. A Void response in the callback would be useful here. How will
this be handled in v2? The Callback method suggested in #287https://github.com/square/retrofit/issues/287looks clean if you leave out the
VoidResponse stuff.


Reply to this email directly or view it on GitHubhttps://github.com//issues/297#issuecomment-22428727
.

@adriancole

This comment has been minimized.

Copy link
Contributor

adriancole commented Aug 22, 2013

Thoughts about Observer

Seems to work well enough for nice json lists from an RPC response (ex [ {}, {}, ..]). A converter might look like this.

  public void intoObserver(TypedInput body, Type type, Observer<? super Object> observer, AtomicBoolean subscribed) throws ConversionException {
      JsonReader jsonReader = new JsonReader(body.in(), charset);
      jsonReader.beginArray();
      while (subscribed.get() && jsonReader.hasNext()) {
        observer.onNext(fromJson(jsonReader, type));
      }
   }

So, this would emit each item in the array to the observer. Cool.. works.. saves reaction time, which could be seconds or longer.

On the other hand, "streaming" apis don't always have nice or similar json forms (think tweet blobs or newline). This could lead to confusing converter code. Further, anytime we touch or even think about IO, we'd have to think about the impact on IncrementalConverter, which if not widely used, could be wasted cycles.

This is compounded if folks interested in Observing things switch to message oriented or multiplexed protocols which have less need to send really long streams of things (which is what the incremental parsing optimizes for).

In short, I'm thinking just supporting Callback in 2.0 would be better than investing in Observer in our base model.

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Aug 23, 2013

Dropped observable from the spec. Added error callback interface (still needs description).

@swankjesse

This comment has been minimized.

Copy link
Member

swankjesse commented Aug 24, 2013

I think the callback can be an abstract class:

public abstract class ResponseCallback<T> {
  public abstract void success(T response);

  public void networkError(IOException e) {
    error(-1, e);
  }
  public void unexpectedError(Exception e) {
    error(-1, e);
  }
  public void httpError(int status, ...) {
    error(status, e);
  }
  public void error(int status, Exception e) {
    log(...);
  }
}

Though not as pure as an interface, this approach is ergonomic. You can handle all error types together, or you can handle each kind independently. This is the approach I'm considering for OkHttp. Whatever we do here, we should probably do similarly in OkHttp.

@swankjesse

This comment has been minimized.

Copy link
Member

swankjesse commented Aug 24, 2013

I think you should rename Response to Call and get to execute, with the symmetric access API @emilsjolander described above.

Call <Tweet> listTweetsCall = api.listTweets("JakeWharton");

// sync:
List<Tweet> tweets = listTweetsCall.execute();

// or async:
listTweetsCall.execute(new Callback<List<Tweet>>() {
  @Override public void onSuccess(List<Tweet> response) { }
});

I prefer execute over get because it's clear that that's when the work is kicked off by the executor thread. With non-action method names like get() or setCallback it's unclear when the executor starts executoring. (And we shouldn't kick off the work in a background thread until we know that the request is async.)

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Apr 29, 2015

OkHttp knows about transport which is much more important. Things like SPDY
and HTTP/2 and even connection reuse on normal HTTP are devalued by not
using its threading.

On Wed, Apr 29, 2015 at 5:22 PM Hannes Dorfmann notifications@github.com
wrote:

Regarding RxJava: is there a reason why not using RxJava Schedulers
instead of OkHttp async threading? Would be nice to be able to specify
subscribeOn() manually (i.E. for unit testing)


Reply to this email directly or view it on GitHub
#297 (comment).

@mwajeeh

This comment has been minimized.

Copy link

mwajeeh commented Apr 30, 2015

@YuriHeupa I know you are asking for a proper cancel mechanism, I am waiting for it too. But for now this works perfectly for me, I am sure you might be doing something similar, if not, have a look at this. Just extend from this NetworkingFragment.

public class NetworkingFragment extends BaseFragment {
    protected final CallbacksManager callbacksManager = new CallbacksManager();

    @Override
    public void onResume() {
        super.onResume();
        callbacksManager.resumeAll();
    }

    @Override
    public void onDestroyView() {
        callbacksManager.pauseAll();
        super.onDestroyView();
    }

    @Override
    public void onDestroy() {
        callbacksManager.cancelAll();
        super.onDestroy();
    }

    protected Service apiFor(CallbacksManager.CancelableCallback<?> callback) {
        callbacksManager.addCallback(callback);
          // return your retrofit API
        return Api.getService();
    }
}

CallBackManager:

public class CallbacksManager {
    private final Set<CancelableCallback> callbacks = new HashSet<>();

    public void cancelAll() {
        for (CancelableCallback callback : callbacks) {
            // false to avoid java.util.ConcurrentModificationException alternatively we can use
            // iterator
            callback.cancel(false);
        }
        callbacks.clear();
    }

    public void resumeAll() {
        final Iterator<CancelableCallback> iterator = callbacks.iterator();
        while (iterator.hasNext()) {
            boolean remove = iterator.next().resume();
            if (remove) {
                iterator.remove();
            }
        }
    }

    public void pauseAll() {
        for (CancelableCallback callback : callbacks) {
            callback.pause();
        }
    }

    public void addCallback(CancelableCallback<?> callback) {
        callbacks.add(callback);
    }

    private void removeCallback(CancelableCallback<?> callback) {
        callbacks.remove(callback);
    }

    public abstract class CancelableCallback<T> implements Callback<T> {
        private boolean canceled;
        private boolean paused;

        private T pendingT;
        private Response pendingResponse;
        private RetrofitError pendingError;

        public CancelableCallback() {
            this.canceled = false;
        }

        public void pause() {
            paused = true;
        }

        public boolean resume() {
            paused = false;
            // if callback was cancelled then no need to post pending results
            if (canceled) {
                return true;
            }
            if (pendingError != null) {
                onFailure(pendingError);
                // to make sure not to post it again
                pendingError = null;
                return true;
            } else if (pendingT != null) {
                onSuccess(pendingT, pendingResponse);
                // to make sure not to post it again
                pendingT = null;
                pendingResponse = null;
                return true;
            }
            return false;
        }

        public void cancel(boolean remove) {
            canceled = true;
            if (remove) {
                removeCallback(this);
            }
        }

        @Override
        public void success(T t, Response response) {
            if (canceled) {
                return;
            }
            if (paused) {
                pendingT = t;
                pendingResponse = response;
                return;
            }
            onSuccess(t, response);
            removeCallback(this);
        }

        @Override
        public void failure(RetrofitError error) {
            if (canceled) {
                return;
            }
            if (paused) {
                pendingError = error;
                return;
            }
            onFailure(error);
            removeCallback(this);
        }

        protected abstract void onSuccess(T t, Response response);

        protected abstract void onFailure(RetrofitError error);
    }
}

Usage:

private void fetchUser(String id) {
    final CallbacksManager.CancelableCallback<User> callback = callbacksManager.new CancelableCallback<User>() {
        @Override
        protected void onSuccess(User user, Response response) {
        }

        @Override
        protected void onFailure(RetrofitError error) {
        }
    };
    // this is only drawback of this approach, you need to pass your callback to your service interface as well as to 
    // Networking fragment for life cycle handling
    apiFor(callback).getUser(id, callback);
}
@onigirisan

This comment has been minimized.

Copy link

onigirisan commented May 23, 2015

@mwajeeh
thank you so much, you save my time.

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Jun 1, 2015

Most of this was done in #845. For everything else there's separate issues tracking it.

@JakeWharton JakeWharton closed this Jun 1, 2015

@Tooto

This comment has been minimized.

Copy link

Tooto commented Jun 10, 2015

when i can get this with graddle?

@YuriHeupa

This comment has been minimized.

Copy link

YuriHeupa commented Jun 11, 2015

@Tooto You can get the 2.0 snapshot from the Sonatype's snapshots repository, just add the following to your gradle file:

repositories {
maven { url "https://oss.sonatype.org/content/repositories/snapshots/" }
}

And declare this on your dependencies:

compile 'com.squareup.retrofit:retrofit:2.0.0-SNAPSHOT'

@jdreesen

This comment has been minimized.

Copy link
Contributor

jdreesen commented Jun 11, 2015

But be aware that the 2.0 API is far from stable and moving fast at the moment ;)

@felipecsl

This comment has been minimized.

Copy link
Contributor

felipecsl commented Jun 16, 2015

is there a separate issue for tracking the error handling implementation?

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Jun 16, 2015

Error handlers can be implemented as CallAdapters. There will be an example.

On Tue, Jun 16, 2015 at 12:58 AM Felipe Lima notifications@github.com
wrote:

is there a separate issue for tracking the error handling implementation?


Reply to this email directly or view it on GitHub
#297 (comment).

@stephanenicolas

This comment has been minimized.

Copy link

stephanenicolas commented Jul 2, 2015

@jdreesen @JakeWharton @YuriHeupa, can the source code of 2.0 preview be seen somewhere ?
Even if it's just experimental..and moving fast.

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Jul 2, 2015

master

On Thu, Jul 2, 2015, 6:10 PM Stéphane Nicolas notifications@github.com
wrote:

@jdreesen https://github.com/jdreesen @JakeWharton
https://github.com/JakeWharton @YuriHeupa https://github.com/YuriHeupa,
can the source code of 2.0 preview be seen somewhere ?
Even if it's just experimental..and moving fast.


Reply to this email directly or view it on GitHub
#297 (comment).

@stephanenicolas

This comment has been minimized.

Copy link

stephanenicolas commented Jul 2, 2015

Wow :) Cool. Thx. Eager to try that.

@changyuheng

This comment has been minimized.

Copy link

changyuheng commented Aug 7, 2015

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Aug 7, 2015

You cite Dagger 2. Google approached us about them spearheading Dagger 2
and we participated in both the design and development of it. You offered
no such interaction and instead eschewed the obvious work being done in
this repo to make some attempt to claim the version 2 of the same name with
a hero feature almost no one is asking for but retains all of the poor API
decisions of our version 1. Your efforts would have been appreciated on
this project instead of silently off in a fork. Unless you change the name,
do not link that project here again.

On Fri, Aug 7, 2015, 2:18 AM Chang Yu-heng notifications@github.com wrote:

See Also: yongjhih/retrofit https://github.com/yongjhih/retrofit


Reply to this email directly or view it on GitHub
#297 (comment).

@yongjhih

This comment has been minimized.

Copy link

yongjhih commented Aug 7, 2015

Sorry. No offense, it's a experimental project. The naming/building were just following Google Dagger 2 without intention. You can see the description that is copied.

Avoid unnecessary politics/ideology issues. I thought to introduce it after some projects tested it, and approach you, ask for advice. It's time. It is at your disposal. What name do you suppose? Or just remove that project.
I don't beg appreciation. I'd like to make a thing right.

@DASAR

This comment has been minimized.

Copy link

DASAR commented Aug 29, 2015

Finally we have Canceling in Retrofit. That's awesome.
How to cancel all currently ongoing requests?
Does it backward compatible with 1.9.x ? (If not, put this info somewhere at the beginning please)

@cooperkong

This comment has been minimized.

Copy link

cooperkong commented Sep 21, 2015

Error handlers can be implemented as CallAdapters. There will be an example.

Where can I find such an example?
Cheers,

@liuxue0905

This comment has been minimized.

Copy link

liuxue0905 commented Oct 19, 2015

[Converter!!!]It's better:
1 Can add Default Converter
2 Can handle Simple Retrun Type / Or supply Simple Converter:Like String,byte,Integer,Stream
3 Can Register Multi Converter for different Return Type

Sometimes,i just want:
Call<String> method(...);
Call<byte[]> method(...);
Call<I/O Stream> method(...);

@samskiter

This comment has been minimized.

Copy link

samskiter commented Nov 10, 2015

Hi, I'm a little confused about what state error handlers at are but I've just been thinking about them in the context of autogenerated code. Would it be perhaps possible to define all the possible (known) paths of success (only one) and error (potentially a few) at compile time? Like defining a subclass of a Callback with the error types I want to handle (HTTP 400, HTTP 404, etc) annotated? so something like:

retrofit:

public interface Callback<T> {

  /** Successful HTTP response. */
  void success(T t, Response response);

  /**
   * Unsuccessful HTTP response due to network failure, non-2XX status code, or unexpected
   * exception.
   */
  void failure(RetrofitError error);
}

custom:

public interface UsersGetEndpointCallback extends Callback<User> {
  @HTTP(400)
  void failure400(My400BodyErrorType error);
}

service:

@GET("/users/{user}")
void getUser(@Path("user") String user, UsersGetEndpointCallback cb);

I've not fully thought through the implications how this would be connected up, but it would offer really good compile time guarantees that I've handled the error cases I care about.

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Nov 10, 2015

You don't need to generate that, just use an abstract class for your
callback, override onSuccess and onFailure, and call the more specific
abstract methods.

On Tue, Nov 10, 2015, 6:16 AM Sam Duke notifications@github.com wrote:

Hi, I'm a little confused about where error handlers are but I've just
been thinking about them in the context of autogenerated code. Would it be
perhaps possible to define all the possible (known) paths of success (only
one) and error (potentially a few) at compile time? Like defining a
subclass of a responseCallback with the errorTypes I want to handle (400,
404, etc) annotated? so something like:

public interface Callback {

/** Successful HTTP response. */
void success(T t, Response response);

/**

  • Unsuccessful HTTP response due to network failure, non-2XX status code, or unexpected
  • exception.
    */
    void failure(RetrofitError error);
    }

public interface UsersGetEndpointCallback extends Callback {
@http(400)
void failure400(My400BodyErrorType error);
}

I've not fully thought through how this would be connected up, but it
would offer really good compile time guarantees that I've handled the error
cases I care about.


Reply to this email directly or view it on GitHub
#297 (comment).

@samskiter

This comment has been minimized.

Copy link

samskiter commented Nov 10, 2015

Sure, I mean that it creates a more type-safe way to ensure errors are handled. I can't use the endpoint unless I handle the errors in a callback object and I get the deserialization of error bodies for free too, reducing boilerplace. Otherwise I end up documenting it:

/*
  Returns 400 is the user object is not get initialized. Body will be the standard JSON error object
  Returns 403 is the user is not authorized.
 */
@GET("/users/{user}")
void getUser(@Path("user") String user, Callback<User> cb);

And then handling the error, checking the errortype, deserializing the json.

When I say autogenerated, I mean from things like RAML or Swagger. These allow error cases to be defined just as thoroughly as success cases.

@ArslanAli7

This comment has been minimized.

Copy link

ArslanAli7 commented Nov 13, 2015

Hi Jack i am facing an issue when i try to run the app for the first time on a new device the first request doesn't execute,i have to send the request again to get the response,i am using retrofit version: retrofit:2.0.0-beta2,and in my code i am send a post request on success of which i am executing the get request,i am not using a custom version of okhttp,i am using the version which is build with retrofit already,i have tested on emulator as well android nexus 6 device.i am using methods like Call.eneque for async processing of request and i am calling these methods on button click without any use of asynctasks...,should i switch back to the previous version or there is issue with my code.waiting for your response, keep up the good work!

@TobiasReich

This comment has been minimized.

Copy link

TobiasReich commented Sep 27, 2016

Hey there,
I really like using Retrofit for my network calls. Still I'm wondering if there is a good way of repeating a call only several times in case there is an error?
The idea is to try the same call for example only 3 times before trying other solutions. The clone method does not allow me to add any further values to it like an int "retriesLeft" or such things.
So is there a good way of solving this issue?
Thanks!

@swankjesse

This comment has been minimized.

Copy link
Member

swankjesse commented Sep 27, 2016

Consider using an OkHttp application interceptor.
https://github.com/square/okhttp/wiki/Interceptors

@sockeqwe

This comment has been minimized.

Copy link

sockeqwe commented Sep 27, 2016

Consider using an OkHttp application interceptor.
https://github.com/square/okhttp/wiki/Interceptors

this

OR

if your are using Retrofit + RxJava you could use retry or retryWhen operator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment