Built-in retry! 🎉

[sunshinejr]

(Note: we should wait with this PR until all the current fixes are merged & released in the next beta, but this is something I want to support soon)

TODO:

• tests
• documentation
• examples (maybe GitHub provider should use a retry closure)

[MoyaBot]

	Warnings
⚠️	Any changes to library code should be reflected in the Changelog. Please consider adding a note there and adhere to the Changelog Guidelines.
⚠️	Sources/Moya/MoyaProvider.swift#L169 - MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...' (mark)
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L121 - immutable value 'error' was never used; consider replacing with '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L121 - immutable value 'error' was never used; consider replacing with '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L154 - initialization of immutable value 'cancellable1' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L154 - initialization of immutable value 'cancellable1' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L167 - initialization of immutable value 'cancellable2' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L167 - initialization of immutable value 'cancellable2' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L215 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L215 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L218 - 'let' pattern has no effect; sub-pattern didn't bind any variables
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L218 - 'let' pattern has no effect; sub-pattern didn't bind any variables
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L262 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L262 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L275 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L275 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L297 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L297 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L310 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it
⚠️	Tests/MoyaTests/MoyaProvider+CombineSpec.swift#L310 - initialization of immutable value 'cancellable' was never used; consider replacing with assignment to '_' or removing it

	Messages
📖	iOS: Executed 292 tests, with 0 failures (0 unexpected) in 14.926 (15.198) seconds
📖	tvOS: Executed 292 tests, with 0 failures (0 unexpected) in 14.941 (15.174) seconds
📖	macOS: Executed 281 tests, with 0 failures (0 unexpected) in 13.908 (14.060) seconds

Generated by 🚫 Danger Swift against a2e91a2

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (development@9862538). Click here to learn what that means.
The diff coverage is 92.85%.

@@              Coverage Diff               @@
##             development    #1924   +/-   ##
==============================================
  Coverage               ?   89.77%           
==============================================
  Files                  ?       30           
  Lines                  ?     1037           
  Branches               ?        0           
==============================================
  Hits                   ?      931           
  Misses                 ?      106           
  Partials               ?        0

Impacted Files	Coverage Δ
Sources/Moya/Plugins/Plugin.swift	75% <ø> (ø)
Sources/Moya/Moya+Alamofire.swift	81.81% <ø> (ø)
Sources/Moya/RequestType.swift	72.22% <0%> (ø)
Sources/Moya/MoyaProvider+Internal.swift	98.59% <100%> (ø)
Sources/Moya/MoyaProvider.swift	91.04% <100%> (ø)
Sources/Moya/MoyaProvider+Defaults.swift	83.87% <100%> (ø)
Sources/Moya/MoyaRequestInterceptor.swift	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9862538...a2e91a2. Read the comment docs.

[amaurydavid]

I've recently struggled using a combination of Alamofire's requestRetrier and requestAdapter with Moya plugins, and finally ended up handling the retrying by myself with embedded moyaprovider.request() calls.
The main issue was that when retrying with the requestRetrier, requestAdapter was used upon retry but Moya plugins were not called. The reason is that for Moya perspective, Alamofire was only performing 1 request even if underhood Alamofire was performing multiple times the request through requestRetrier.
Note: tested on AF 4, I don't know if it still works the same way with AF 5, but I think requestRetrier and requestAdapter had their share of changes.

Having a built-in retry feature would allow plugins to be called upon every retry, which is nice.

There are some points I would like to raise, though it may be best to discuss them in a dedicated thread:

• If we provide a retry feature, we will want to make sure it's not possible to use Moya's and Alamofire's at the same time. At the moment, it's possible to access the Alamofire's sessionManager using moyaProvider.manager, maybe we should make the property internal? The drawback is that we would have to provide a proxy for any property not directly handled by Moya.

• Do we have a list of Alamofire's features overriden by Moya? It may be a good idea to keep one up to date. I believe the status code validation can also be overriden according to user's preference.
  If we end up overriding too many of Alamofire's features, may it would be time to think about not relying on Alamofire? It may be related to issue [Suggestion] Simpler version of Moya? #1639 at some point.

[sunshinejr]

Hey @amaurydavid - great points. Generally you shouldn't need to use the session at all, we try to provide as much flexibility as possible but if something's not there you can just customize the session (or sessionManager in Moya < 14).

As for the list there is nothing like that AFAIK.

The resolution of #1639 was that we don't really want to remove Alamofire as a dependency due to the fact that we would need to provide our own networking layer (which Moya shouldn't really care about as we are just a layer on top of that).

But this discussion was year ago and maybe we could try again? Maye instead of removing Alamofire (and using our own networking layer in Moya) we could to write a small library under Moya organization that would handle networking? Not sure if we have the capacity for that.

[adrianod1as]

+1

[iWECon]

Why no merger?