Enable managerModifyRequest to modify redirectCount

[soenkehahn]

This changes the behavior of httpRaw and httpRaw': They will not apply mModifyRequest on given requests. (Since they are only exported in Internal modules I hope this is ok.)

I'm not very happy with this code (apart from the test case). How the request is modified has gotten very confusing. This is also very visible in the confusing non-informative names for the requests (req0, req, req', req'' and now inputReq). I'm not very familiar with this code internally, so I'm not sure how to refactor things without breaking other features. I may look into this in the next days, but any pointers (or code) would be appreciated.

One more concrete question: Does it even makes sense that httpRaw' returns a Request if it doesn't apply mModifyRequest to it anymore?

[snoyberg]

I'm currently halfway through a refactoring for http-client 0.5. It doesn't exactly simplify this bit of logic, but it does touch on it. Would you mind holding off until that refactoring is complete and then reapplying this PR to the new master?

[soenkehahn]

Yes, sounds good.

[snoyberg]

Alright, I've merged to master.

[soenkehahn]

I've rebased my branch and changed it, adding some more tests and doing things slightly differently. I'd appreciate any feedback.

There's also a few questions where I don't know how we want http-client to behave. E.g.:

• What if a Request wants to override the Manager with requestManagerOverride? But the Manager also wants to change the Request with managerModifyRequest, possibly changing requestManagerOverride? Who wins? (Maybe this can largely be ignored as a misuse of the library.)
• Is managerModifyRequest meant to be executed once for every sent request if redirects are being followed? (This PR currently executes it twice for the first request unfortunately.)
• Is it important whether mSetProxy is executed before or after managerModifyRequest? This PR changes the order.

cc: @snoyberg

[snoyberg]

Looks OK to me. I agree that the competing features have made things a little convoluted here. How about taking a simplification pass like so:

1. Define the expected behavior clearly. IMO, this would probably be something like "let the Request override the Manager first, then use the Manager to modify the Request, and then proceed".
2. Capture this logic in a comment and a single function of type (something like) getModifiedRequestManager :: Request -> Manager -> IO (Request, Manager).
3. Use that function in all relevant places instead of sprinkling it (as I have until now) between multiple functions

[soenkehahn]

@snoyberg: I've tried to implement your suggestion and rebased on current master.

[soenkehahn]

@snoyberg: Fixed the travis failures.

[snoyberg]

Thanks!

[soenkehahn]

Thanks for merging and the release!