1

A large update and partial rewrite of the Alamofire Usage documentation in now in PR. You can also see the rendered markdown here. It would be great to get some community feedback, either here or on the PR, about what an introductory document should like for the library. It serves as an introduction to the top level APIs, including making requests and handling responses. Any feedback would be great!

2 Likes
2

Thanks Jon, I'm reading it right now.

public enum HTTPMethod: String { case connect, delete, ... }

[...] If you need to use an HTTP method that Alamofire's HTTPMethod type doesn't support, you can still set the String httpMethod property on URLRequest directly.

The closed enum was surprising (because I know there are exotic servers), but it is the sentence that had me write this message.

Is there a reason why HTTPMethod is defined as a closed enum instead of a raw representable type which accepts any string? It would allow Alamofire to support all HTTP methods out of the box, without making the life of users that remain in the strict context of RFC 7231 any more difficult:

public struct HTTPMethod: RawRepresentable {
    public var rawValue: String
    // Let users define their own methods
    public init(rawValue: String) { self.rawValue = rawValue }
    public static let connect = HTTPMethod(rawValue: "CONNECT")
    public static let delete = HTTPMethod(rawValue: "DELETE")
    ...
}

This would avoid the kind of little inconsistencies that makes the life of developers miserable: when a tiny change (supporting an exotic HTTP method) requires heavy refactoring (we can no longer rely on the HTTPMethod type in our stack).

Edit

A few lines below, I have my answer with the ParameterEncoder protocol.

With my naive suggestion, exotic methods would sometimes have to raise a fatal error:

extension HTTPMethod {
    static let exotic = HTTPMethod(rawValue: "EXOTIC")
}

// Fatal error: Please provide an explicit encoder,
// such as URLEncodedFormParameterEncoder(destination: .httpBody)
AF.request(
    "https://httpbin.org/get", 
    method: .exotic, 
    parameters: parameters)

I don't know if Alamofire makes a liberal use of fatalError and preconditions in order to spot programmer mistakes (an ever-hot topic), so I won't elaborate more. I just want to say that in my humble opinion, this is still better than the closed enum, because of my argument above (a tiny change in an app shouldn't require heavy refactoring).

3

You're right. HTTPMethod is one of the original types of Alamofire and hasn't changed much in that time. Creating a fully fleshed out type would probably be fairly simple, but like HTTP headers, I would likely want to create a case-insensitive implementation that allowed get and GET to be the same method. And yes, as you noted, it would make some of our method checks more complex.

In the meantime, switching from HTTPMethod to String in any compositional APIs should allow users to use any headers they want, while still being able to use the HTTPMethod values.

Any other feedback?

4

@gwendal.roue I've created a PR to unfreeze HTTPMethod. Turns out it wasn't much of an issue and should lead to more flexibility.

Alamofire 5 has removed all inadvertent assertions (force unwraps) but making the appropriate APIs throwing, but we do still use a few preconditions internally to ensure the internal state we use to track Request lifetimes is always consistent.

5

Thanks Jon! I'm happy it was a simple change :-) Reading this doc has removed all my questions :+1:

I have another question regarding the robustness of auto-starting requests: I have witnessed in a couple occasions a race condition that happens in Alamofire 4 and the first betas of Alamofire 5:

AF.request(...)
    .validate(...) // too late, response may already have been received
    .responseXXX { ... } // process unvalidated response 😨

I now always set the startRequestsImmediately session flag to false, and always start my requests after they have been fully defined. This prevents the race completely.

I have to write my concern more precisely, in a proper Github issue. But does what I say ring a bell to you?

6

@Jon_Shier, actually both Feature - Appending Response Serializers by cnoon · Pull Request #2862 · Alamofire/Alamofire · GitHub and Feature - Terminal Response Serializers by cnoon · Pull Request #2810 · Alamofire/Alamofire · GitHub (especially #2862) look like they are related. But I'm still wondering if the only way to get the truly non-brainer Alamofire everybody wants is to set startRequestsImmediately to false. "Once bitten, twice shy" say some people :sweat_smile:

7

The race between adding a validator and processing a response is still there, but should now be much more rare due to the entire request pipeline being async in Alamofire 5. Adding a validator now locks the Request's internal state while it's being added, which means it should usually win the race. But yes, these sorts of APIs are inherently racy, and it's always been our recommendation that anyone encountering issues turn off automatic start.

Automatic start was one of the original reasons for the creation of Alamofire, so for better or worse it's an expected part of the library now. I think we've fixed most of the issues people have seen with the internal rewrite that's part of Alamofire 5.

8

Thanks Jon. That's very clear. I'll preventively keep pushing on the safe handle, then :slight_smile:

No more comment on the great Alamofire 5! Let's have other people chime in!

9

The Usage documentation update has landed on master, take a look!

Also, @gwendal.roue, HTTPMethod has been unlocked.

This will all be released as Alamofire 5.0.0-rc.1 later today or tomorrow.

2 Likes
10

Hi!

The Usage.md looks pretty good, but the AdvancedUsage.md seems to be outdated.

It talks about using the SessionManager (I was specifically looking how to use adapters and retriers) but there is no SessionManger anymore. Any when trying to change the adapter of the Session object, the compiler complains about it being 'internal'

Any updated docs on this matter would be appreciated

Thanks!

11

Yes, you can no longer mutate a Session, you must initialize it with the appropriate internal state. RequestInterceptors can also be passed on a per-request basis.

We’re working on updating the advanced docs now, it’s just a huge amount of documentation to rewrite.

2 Likes
12

Where can we see the rendered markdown now? Looks like the link in the OP is broken.

13

It’s now merged into master.

1 Like
14

Found it, thanks. Migration guide to come at a later date?

15

Yes, we’re working on it now. Due to extensive changes in v5, it’s rather long.

1 Like
16

How does Combine avoid these kind of issues, especially session.dataTaskPublisher?

I‘m guessing that it has something to do with the publisher only sending data once a subscriber subscribed and the individual operators not actually subscribing until they are subscribed to?

Would something like that be possible for Alamofire? Like, only auto-start the request if a response handler was attached. And for those few cases where people want to actually send a request but not look at the response at all, make a no-op response handler called fireAndForget.

It could look like this:

AF.request(...).fireAndForget()
17

That's a very good idea.

I guess you mean disable auto-start in Alamofire task publishers, though. The Combine subscription would explicitly start the request only after it has been fully set up, avoiding all races for good.

18

I haven’t looked at the details of Alamofire 5 yet and whether it actually uses Combine Publishers or not, so I might be misunderstanding this. But my comment was referring to setting up the requests in general, not just Combine-conformant parts.

Yes, in effect, my comment means „disable auto-start overall“. But attaching a response handler would start the request (by default) which would still make it feel like auto-start.

19

Yes @cocoafrog, we absolutely agree. I'm sorry if I sounded pedantic. Alamofire 5 has no documented support for Combine yet, but we can be assured it will ship eventually.

20

Starting when the first handler is attached is an interesting suggestion, I don't know if we'd considered that before. I'm not sure it's a change we can make for v5, as users may be relying on the behavior, and we're already into the Release Candidates. I'm not sure it's a legitimate use case, but there's no way of know if anyone uses the library that way.

Our Combine implementation avoids the issue by not even creating the Request until the publisher receives a subscription, at which point everything should be in place to handle the response. However, I haven't examined how the Combine APIs will interact with the additive closure APIs that Alamofire has, so I'm not sure how it will all play out yet.

I'll definitely look into starting requests when the first response handler is added though.