haf / Http.fs

A simple, functional HTTP client library for F#
323 stars 41 forks source link

Changed from use of HttpWebRequest to HttpClient #134

Closed seanamos closed 7 years ago

seanamos commented 7 years ago

Resolves: https://github.com/haf/Http.fs/issues/132

The reason this is in WIP is that there are breaking changes and cleanup needs to be done. I've commented out surface area which there is not an easy way to "map" to HttpClient, or made significant changes from the original. HttpClient moves a lot of settings into HttpMessageHandler, which is instantiated once and shared across requests.

haf commented 7 years ago

Looking good generally. Looking forward to seeing the future of this PR.

seanamos commented 7 years ago

I think I've figured out why Suave throws and drops the connection when manually writing the StreamContent.

TCP request processing failed
System.Exception: invalid url: ''
   at <StartupCode$Suave>.$ConnectionFacade.processRequest@439-2.Invoke(String _arg29)
   at Microsoft.FSharp.Control.AsyncBuilderImpl.args@823-1.Invoke(a a)

Manually written:

POST http://localhost:8080/gifs/echo HTTP/1.1
Connection: Keep-Alive
Content-Type: multipart/form-data; boundary="XP--OOmGuPlNTOczjtmYwqT+Rprucx"
Access-Code: Super-Secret
Content-Length: 249
Host: localhost:8080

--XP--OOmGuPlNTOczjtmYwqT+Rprucx
Content-Disposition: form-data; name="img"; filename="pix.gif"
Content-Type: image/gif
Content-Transfer-Encoding: binary

GIF89a  ‘     ÿÿÿÿÿÿ   !ù   ,       D ;
--XP--OOmGuPlNTOczjtmYwqT+Rprucx--

Using .NETs various content types, in this case ByteArrayContent:

POST http://localhost:8080/gifs/echo HTTP/1.1
Connection: Keep-Alive
Content-Type: multipart/form-data; boundary="kNxUqLJiWyXWZR+-LvTdeedg/R'KVV"
Access-Code: Super-Secret
Content-Length: 247
Host: localhost:8080

--kNxUqLJiWyXWZR+-LvTdeedg/R'KVV
Content-Transfer-Encoding: binary
Content-Disposition: form-data; name="img"; filename="pix.gif"
Content-Type: image/gif

GIF89a  ‘     ÿÿÿÿÿÿ   !ù   ,       D ;
--kNxUqLJiWyXWZR+-LvTdeedg/R'KVV--

Subtle but the Content-Length might give it away. It's the extra whitespace on the end.

seanamos commented 7 years ago

Travis failure is just mono randomly blowing up in paket, can see in the matrix it does actually pass.

haf commented 7 years ago

All green. So, I guess we're merge-ready now?

seanamos commented 7 years ago

I think so, I've been testing it internally for some basic things in .NET Core 2 apps (mostly simple json get/post) and it's been working fine so far.

haf commented 7 years ago

Thank you very much! I'll play with this locally a bit and may ping you going forward to get this released.

seanamos commented 7 years ago

Looking forward to it!

ivpadim commented 7 years ago

Hi everyone, I've been playing around with the netstandard version and I believe I have an issue with a Content-Type not getting set properly, on line:

https://github.com/haf/Http.fs/blob/master/HttpFs/HttpFs.fs#L878

The content-type I'm getting from the response is

I_OBJECT; charset=utf-8

Because there is no ContentType I'm getting a NullReferenceException.

This is my first time here, Should I open an issue?

seanamos commented 7 years ago

@ivpadim I'll take a look, shouldn't be too difficult to guard against this

ivpadim commented 7 years ago

@seanamos thanks, I'm having the same issue when I get a response with no content-type header. The website is a SAP portal and for what I can tell is pretty old. BTW with version 4.1.2 is working nicely!