Skip to content
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

cohttp-eio: client: use permissive argument type for make_generic #1026

Merged

Commits on Apr 4, 2024

  1. cohttp-eio: client: use permissive argument type for make_generic

    Currently, Cohttp_eio.Client.make_generic takes a function that returns a value
    of type `_ Eio.Net.stream_socket` as an argument. This type is too strict and
    can be relaxed to `_ Eio.Flow.two_way`. The difference between these two
    types is actually important when we use cohttp-eio with ocaml-tls. Consider the
    following code:
    
    ```
    let authenticator = Ca_certs.authenticator () |> Result.get_ok
    
    let connect_via_tls url socket =
      let tls_config = Tls.Config.client ~authenticator () in
      let host =
        Uri.host url
        |> Option.map (fun x -> Domain_name.(host_exn (of_string_exn x)))
      in
      Tls_eio.client_of_flow ?host tls_config socket
    
    let connect net ~sw url =
      (* NOTE: Do something different than `Cohttp_eio.Client.tcp_address` here and get `addr` *)
      let socket = Eio.Net.connect ~sw net addr in
      connect_via_tls url socket
    
    let () =
      Eio_main.run @@ fun env ->
      Cohttp_eio.Client.make_generic (fun ~sw url ->
          let flow = connect (Eio.Stdenv.net env) ~sw url in
          flow (* <<---- TYPE ERROR HERE!
    
    Error: This expression has type
             Tls_eio.t = [ `Flow | `R | `Shutdown | `Tls | `W ] Eio_unix.source
           but an expression was expected of type
             [> [> `Generic ] Eio.Net.stream_socket_ty ] Eio_unix.source
           Type [ `Flow | `R | `Shutdown | `Tls | `W ] is not compatible with type
             [> ([> `Generic ] as 'a) Eio.Net.stream_socket_ty ] =
               [> `Close
                | `Flow
                | `Platform of 'a
                | `R
                | `Shutdown
                | `Socket
                | `Stream
                | `W ]
           The first variant type does not allow tag(s)
           `Close, `Platform, `Socket, `Stream
    *))
    ```
    
    This code won't compile if `make_generic` expects `_ Eio.Net.stream_socket`,
    but it will compile if `_ Eio.Flow.two_way`.
    
    This patch solves the above problem by changing the interface of `make_generic`
    to `(sw:Switch.t -> Uri.t -> _ Eio.Flow.two_way) -> t`. The implementation
    of `make_generic` is effectively an identity function, so we don't need to
    change the implementation.
    ushitora-anqou committed Apr 4, 2024
    Configuration menu
    Copy the full SHA
    539db5c View commit details
    Browse the repository at this point in the history