haskell-servant / servant

Servat is a Haskell DSL for describing, serving, querying, mocking, documenting web applications and more!
https://docs.servant.dev/
1.81k stars 409 forks source link

client streaming + http redirect result into runtime exception #1091

Open domenkozar opened 5 years ago

domenkozar commented 5 years ago

If an http endpoint returns a redirect (for example http -> https upgrade), streaming client errors out:

HasClient @StreamBody
CallStack (from HasCallStack):
  error, called at src/Servant/Client/Core/Internal/HasClient.hs:550:44 in servant-client-core-0.15-KKVc3oxuLh47HhicA3CWtB:Servant.Client.Core.Internal.HasClient                                                                                                                                                     

https://github.com/haskell-servant/servant/blob/master/servant-client-core/src/Servant/Client/Core/Internal/HasClient.hs#L550

phadej commented 5 years ago

Looks like it errors out always. I forgot to write an implementation :/

Sent from my iPhone

On 28 Nov 2018, at 16.08, Domen Kožar notifications@github.com wrote:

If an http endpoint returns a redirect (for example http -> https upgrade), streaming client errors out:

HasClient @StreamBody CallStack (from HasCallStack): error, called at src/Servant/Client/Core/Internal/HasClient.hs:550:44 in servant-client-core-0.15-KKVc3oxuLh47HhicA3CWtB:Servant.Client.Core.Internal.HasClient
https://github.com/haskell-servant/servant/blob/master/servant-client-core/src/Servant/Client/Core/Internal/HasClient.hs#L550

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

phadej commented 5 years ago

I won't be able to look at this issue this week. But if someone can make a patch, then I'll happily review it. (and make the release after merge).

domenkozar commented 5 years ago

There's absolutely no hurry from my side.

domenkozar commented 5 years ago

This is now blocking https://github.com/cachix/cachix/pull/163

domenkozar commented 5 years ago

Note that this has to be reopened, we haven't fixed the redirect use/test case yet.

phadej commented 5 years ago

Streaming + redirect ought to use something like https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/100

Otherwise I don't see how to stream body only once to final target

phadej commented 5 years ago

I relabel this as feature, as the bug part is fixed now.

(I discovered this as http-streams has 100 code handling, I don't know about http-client, and warp (related Expect: 100-continue))