Issue #588: allow a function supplying the agent to support in follow…

[edgraaff]

This PR addresses issue #588. When the user wants to provide an agent option, the user has to decide (prior making the call to fetch) to provide a http or https agent. If request is for http and makes it redirect to https (or vice versa), the provided agent is incompatible and breaks further processing.
Based on the proposal, agent can now be a function which takes parsedURL as an argument.

A typical use would be:

const httpAgent = new http.Agent({ keepAlive: true });
const httpsAgent = new https.Agent({ keepAlive: true });

const options = {
  agent: url => url.protocol === 'https:' ? httpsAgent : httpAgent
};

[bitinn]

This looks good to me, solves issues related to url-based agent creation (not just http/https change during redirect, but more advanced management of connection pools too.)

@jimmywarting what's your take on this?

[jimmywarting]

Lgtm

[jimmywarting]

From the small readme changed it's unclear what argument the function takes and what you should return.

Aside from that I would almost want to get rid of the option to provide a agent and only have a callback function

- agent: null   // http(s).Agent instance (or function providing one), allows custom proxy, certificate, dns lookup etc.
+ agent: null   // function(url) { ... } that can return an Agent that allows for custom proxy, certificate, dns lookup etc.

Think it's better to have one way of solving things instead of having to deal with every scenario

[bitinn]

Aside from that I would almost want to get rid of the option to provide a agent and only have a callback function

That's a breaking change and I think largely an unnecessary break (saves only 2 lines).

PR welcomed to fix doc though. I prefer adding another paragraph below the code block.

[jimmywarting]

That's a breaking change and I think largely an unnecessary

Yea, i know and that would be bad. Agree that it's unnecessary.

PR welcomed to fix doc though. I prefer adding another paragraph below the code block.

or just an example block using agent? didn't see any in the readme...

---

I'm just a bit frustrated ATM on another project i'm refactoring. it tries to deal with a lot of unnecessary overhead instead of just accepting one form of input. kinda: is stream? is it a buffer? canvas? string? promise? callback function? object? blob? FileList? filePath? url? then do this or that and it magically unwrap itself sometimes sync and sometimes async

Now when i mention it, it kind sounds like our Body.js constructor 😅
but it's not node-fetch i'm lashing out on. my other project is way worse