-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Feature - Request Adapter and Retrier System #1450
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
Conversation
cc @jshier and @kcharwood. I'd love to get your guys thoughts here. |
c9c1fdc
to
e225359
Compare
|
||
#### RequestRetrier | ||
|
||
The `RequestRetrier` protocol allows a `Request` that encountered an `Error` while being executed to be retried. When using both the `RequestAdapter` and `RequestRetrier` protocols together, you can create credential refresh systems for OAuth1, OAuth2, Basic Auth and even exponential backoff retry policies. The possibilities are endless. Here's a short example of how you could implement a refresh flow for OAuth2 access tokens. |
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.
Seems like there should be a simpler example or walkthrough of the RequestRetrier
protocol before jumping directly into an OAuth2 implementation. From looking at it I can't tell what's actually required for the protocol here and what's part of the OAuth2 flow.
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.
@jshier Quick Opinion, the name RequestAdapter makes me think on the Adapter pattern in GoF's Design Patterns. I understand why you pick the name, but I think the pattern you are using is more commonly referred to as the Interceptor pattern. So RequestInterceptor might be a better name.
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.
I agree @jshier...a simpler example is definitely warranted. I'll make a note to try to get something put together when we rework all the docs. @tobiasoleary I respectfully disagree with the naming. An interceptor
doesn't imply the Request
will be modified in any way.
@cnoon Initial review complete. We discussed some of the general issues I had but it looks great otherwise! |
Thanks for all the awesome comments here @jshier. Everything has been addressed at this point so I'm going to merge it in to keep the ball rollin'. 👍🏼 |
@cnoon how to intercept response ? exactly like Adaptor for response. |
You have a couple of places you could do that @alizainprasla, the |
@cnoon Wouldn't be nice the
Does it make sense? Should I do a PR? |
@ricardopereira It would be much easier to just allow adapter errors to trigger retry behavior. I'm not sure if that would work now, but it might become possible for Alamofire 5. Such a feature likely wouldn't ship until then anyway. |
@jshier That sounds even better, would be simpler to reuse the logic behind reauthentication. Should I open a Feature request? |
Sure. At the very least it will give us a fresh issues to discuss adaptation vs. retry. |
@ricardopereira and @jshier, This can actually already be done on AF4.x. I know b/c we're doing it in our internal Nike networking library. How?We throw custom |
@cnoon Thanks for sharing! I'll try doing the same. |
This PR adds a couple powerful protocols,
RequestAdapter
andRequestRetrier
, that open up all sorts of possibilities including:Request
modifications forAuthorization
headersI'm sure there are all sorts of other cool things you could do with this, but these are the main things I had in mind when putting this together.
RequestAdapter
The
RequestAdapter
protocol allows eachRequest
made on aSessionManager
to be inspected and adapted before being created. One very specific way to use an adapter is to append anAuthorization
header to requests behind a certain type of authentication.RequestRetrier
The
RequestRetrier
protocol allows aRequest
that encountered anError
while being executed to be retried. When using both theRequestAdapter
andRequestRetrier
protocols together, you can create credential refresh systems for OAuth1, OAuth2, Basic Auth and even exponential backoff retry policies. The possibilities are endless. Here's a short example of how you could implement a refresh flow for OAuth2 access tokens.Tests
I've added tests around the
RequestAdapter
to make sure all theSessionManager
APIs actually call theadapter
if set. I also added tests verifying theretrier
is called when errors occur and that theadapter
will be called again when the request is retried.Internal Modifications
There are several internal changes that are worth calling out to make this work.
Validations
The first is that
Validation
is no longer run on the delegate'sOperationQueue
. In order to determine whether aRequest
encountered an error, we need to make sure we run all the validations first, otherwise the error won't be set and theretrier
won't be called. This was a trivial change to implement, but is an important callout.TaskConvertible
The
TaskConvertible
enum nested in theRequest
class allows us to store the un-adapted version of theRequest
before it is adapted and turned into aURLSessionTask
. Theretry
method on theSessionManager
uses the neworiginalTask
property to extract the original urlRequest, adapt it if necessary, then create the new task and apply it to theRequest
. By setting a new task on theTaskDelegate
, it automatically resets all the data tracked in theTaskDelegate
as though it is a brand new task.SessionDelegate
The
SessionManager
must be passed down to theSessionDelegate
as aweak
property in order to allow theSessionManager
to retry theRequest
from theSessionDelegate
. I originally had the retry logic in theRequest
class, but it became problematic when trying to figure out how to lock down the task creation on theSessionManager
queue. While I don't love having to store the parent reference in the child, it's not so bad since it's not exposed publicly and it doesn't result in a retain cycle since it's aweak
reference.README Updates
I've added a
Adapting and Retrying Requests
section to the README to walk users through some of the ways you could use these protocols. I also created a fairly detailedOAuth2
example to demonstrate how to start to build a thread-safe refresh system that could be shared across multiple session managers.Summary
Overall I think these are two very powerful protocols that will open up all sorts of possibilities for the Alamofire community. I'd love to see custom
RequestRetrier
implementations for linear and exponential back-off policies for things like background sync operations. Can't wait to see what everyone comes up with!