-
Notifications
You must be signed in to change notification settings - Fork 174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix body not being uploaded with unchunked Async.Pipe. #706
Conversation
The CI is red but I'm under the impression it's about the setups themselves ? For instance I'm seeing
|
@@ -128,20 +128,19 @@ let callv ?interrupt ?ssl_config uri reqs = | |||
let call ?interrupt ?ssl_config ?headers ?(chunked=false) ?(body=`Empty) meth uri = | |||
(* Create a request, then make the request. Figure out an appropriate | |||
transfer encoding *) | |||
let req = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is easier to read if you don't pipe such a large block. I'd suggest
let (req, body') = match chunked with
[...]
in
req >>= request ?interrupt ?ssl_config ~body:body' ~uri
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had another look at the code, similar blocks appear in other places. I am going to merge as is
I agree that the CI failures seem all unrelated. If you make the little stylistic change above, I'll be happy to merge |
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…lwt, cohttp-lwt-unix, cohttp-lwt-unix-ssl, cohttp-top, cohttp-async and cohttp-mirage (3.0.0) CHANGES: - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl), and includes ssl in cohttp-async (@dinosaure mirage/ocaml-cohttp#692) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686)
…ohttp-top, cohttp-async and cohttp-mirage (4.0.0) CHANGES: - cohttp.response: fix malformed status header for custom status codes (@mseri @aalekseyev mirage/ocaml-cohttp#752) - Remove dependency to base (@samoht mirage/ocaml-cohttp#745) - fix opam files and dependencies - add GitHub Actions workflow (@smorimoto mirage/ocaml-cohttp#739) - lwt_jsoo: Forward exceptions to caller when response is null (@mefyl mirage/ocaml-cohttp#738) - Remove wrapped false (@rgrinberg mirage/ocaml-cohttp#734) - Use implicit executable dependency for generate.exe (@TheLortex mirage/ocaml-cohttp#735) - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686) - [reverted] breaking changes to client and server API to use conduit 3.0.0 (@dinosaure mirage/ocaml-cohttp#692). However, as the design discussion did not reach consensus, these changes were reverted to preserve better compatibility with existing cohttp users. (mirage/ocaml-cohttp#741, @samoht)
…ohttp-top, cohttp-async and cohttp-mirage (4.0.0) CHANGES: - cohttp.response: fix malformed status header for custom status codes (@mseri @aalekseyev mirage/ocaml-cohttp#752) - Remove dependency to base (@samoht mirage/ocaml-cohttp#745) - fix opam files and dependencies - add GitHub Actions workflow (@smorimoto mirage/ocaml-cohttp#739) - lwt_jsoo: Forward exceptions to caller when response is null (@mefyl mirage/ocaml-cohttp#738) - Remove wrapped false (@rgrinberg mirage/ocaml-cohttp#734) - Use implicit executable dependency for generate.exe (@TheLortex mirage/ocaml-cohttp#735) - cohttp: update HTTP codes (@emillon mirage/ocaml-cohttp#711) - cohttp: add Uti.t to uri scheme (@brendanlong mirage/ocaml-cohttp#707) - cohttp: fix chunked encoding of empty body (@mefyl mirage/ocaml-cohttp#715) - cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl mirage/ocaml-cohttp#706) - cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni mirage/ocaml-cohttp#714 mirage/ocaml-cohttp#712 mirage/ocaml-cohttp#713) - cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri mirage/ocaml-cohttp#717) - refactoring of tests (@mseri mirage/ocaml-cohttp#709, @dinosaure mirage/ocaml-cohttp#692) - update documentation (@dinosaure mirage/ocaml-cohttp#716, @mseri mirage/ocaml-cohttp#720) - cohttp: fix transfer-encoding ordering in headers (@mseri mirage/ocaml-cohttp#721) - lower-level support for long-running cohttp-async connections (@brendanlong mirage/ocaml-cohttp#704) - fix deadlock in logging (@dinosaure mirage/ocaml-cohttp#722) - add of_form and to_form functions to body (@seliopou mirage/ocaml-cohttp#440, @mseri mirage/ocaml-cohttp#723) - cohttp-lwt: partly inline read_response, fix body stream leak (@madroach @dinosaure mirage/ocaml-cohttp#696) - improve media type parsing (@seliopou mirage/ocaml-cohttp#542, @dinosaure mirage/ocaml-cohttp#725) - add comparison functions for Request.t and Response.t via ppx_compare (@msaffer-js @dinosaure mirage/ocaml-cohttp#686) - [reverted] breaking changes to client and server API to use conduit 3.0.0 (@dinosaure mirage/ocaml-cohttp#692). However, as the design discussion did not reach consensus, these changes were reverted to preserve better compatibility with existing cohttp users. (mirage/ocaml-cohttp#741, @samoht)
Body_raw.disable_chunked_encoding
would drain the pipe to get the content-length, but discard the read body, leaving it empty.