Conduit 3 API feedback/questions

[talex5]

I was asked to try updating stuff to conduit 3 and report any questions/problems I have. I'll try to keep track of them here.

Endpoint

The Endpoint module seems very confusing. It defines an endpoint as a host name or an IP address, without a port number.

There is no direct way to pass a port to resolve. The example at https://mirage.github.io/ocaml-conduit/conduit-lwt/Conduit_lwt/index.html#resolution defines an http_resolver that always fixes port as 80.

Looking in the cohttp code to see how a library could use this (https://github.com/mirage/ocaml-cohttp/blob/4b834d57ab24b94791be9b4b68e228a83139d85a/cohttp-lwt-unix/src/net.ml#L45) it seems that cohttp takes a list of resolvers from the user, looks at the URI, and then overrides the user's resolvers with its own ones that set the port to what the user requested! In particular, it overrides the user's TLS configuration settings while doing this!

On the plus side, from the code it looks like certificate checks now default to "on", which is a major improvement!

I suspect that users will either want to supply the configuration directly (using connect instead of resolve) or connect using a URI rather than a domain name. Perhaps the endpoint and resolver stuff should be moved to a separate library, to simplify the conduit API?

Tutorial

I read the tutorial at https://mirage.github.io/ocaml-conduit/conduit/howto.html.

I found this a bit frustrating because it starts by assuming the existence of a conduit-aware getline function, which is never defined. Therefore it is not possible for the user to run the example.

It says that getline can be implemented as well with Conduit.S.recv. However, Conduit_lwt.TCP (the implementation the user is told to use) says:

NOTE: recv wants to fill the given buffer as much as possible until it has reached end-of-input. In other words, recv can do a multiple call to Lwt_unix.recv to fill the given buffer.

Therefore, it is not possible to use recv to input a line, unless you read the data one byte at a time.

(and when I tried it, it returned that it had read 0 bytes of data for some reason)

I think it would also help with the tutorial to start by giving the typical OO API from other languages, and then explain conduit's improvements over that. e.g. something like:

class type flow = object
  method recv : input -> ([ `Input of int | `End_of_flow ], [> error ]) result io
  method send : output -> (int, [> error ]) result io
  method close : (unit, [> error ]) result io
  method cast : 'a ty -> 'a option
end

Otherwise, it's easy to get lost with all the open types, type witnesses, etc, before you even understand what conduit is trying to do.

Client-side conduits

Conduit.S.scheduler defines type scheduler as abstract (https://mirage.github.io/ocaml-conduit/conduit/Conduit/module-type-S/index.html#type-scheduler). This doesn't appear to be used anywhere else in the API. It can probably be removed.

type error is defined as either Msg or Not_found. It's not clear what Not_found means in this context. In particular, recv, send and close all say they can return this.

It would be good to specify the behaviour of these functions more precisely. In particular:

• State whether recv does a single read or tries to read its buffer (should be single IMHO, although TCP is perhaps not doing this at the moment).
• Likewise, define what send does (one call or many). mirage-flow says "There is no indication when the buffer has actually been read and, therefore, it must not be reused". It would be good to clarify whether or not conduit has this restriction.
• close says Subsequent calls to recv will return Ok `End_of_flow. How do I shut down just the sending side and then wait for the response?

It's also not clear to me why these functions are defined as "client-side", since it seems they would be used server-side too.

What does register ~protocol do? Where is it registered?

I got a bit lost with the protocols and services stuff. I guess this is so that I can pass e.g. TCP or TLS to something and have it connect/accept by itself. But since it will have to provide the port and, for TLS, security information, it seems that this can't be quite generic.

For a server socket, it seems like the code that chooses the protocol and service might as well just go ahead and create the listening socket with the appropriate configuration too.

The function

val init : 'cfg -> service:('cfg, 't, 'flow) service -> ('t, [> error ]) result io

seems to boil down to "Give me an argument and a function and I'll call it for you".

Initial thoughts on updating stuff

prometheus-app

The diff is:

-    let mode = `TCP (`Port port) in
-    let thread = Cohttp_lwt_unix.Server.create ~mode (Cohttp_lwt_unix.Server.make ~callback ()) in
+    let sockaddr = Lwt_unix.ADDR_INET (Unix.inet_addr_any, port) in
+    let cfg = { Conduit_lwt.TCP.sockaddr; capacity = 128 } in
+    let thread = Cohttp_lwt_unix.Server.create cfg Conduit_lwt.TCP.protocol Conduit_lwt.TCP.service
+        (Cohttp_lwt_unix.Server.make ~callback ()) () in

• It would be nice to have a construction function for Conduit_lwt.TCP.configuration. Then it could supply defaults for capacity and the bind address, and not break if we add new fields later.
• Not sure why there's an extra () on Server.create.
• It appears that create ignores the backlog unless the service happens to be TCP.

I think it might be better if the listen/accept loop moved to conduit itself. e.g. something like:

let cfg = Conduit_lwt.TCP.config ~port () in
Conduit_lwt.TCP.serve cfg (Cohttp_lwt_unix.Server.make ~callback ())

Hmm, there is a Conduit_lwt.serve. But it returns a Lwt_condition.t to stop it. That's racy - if you broadcast on stop before the loop calls wait then it will miss the event! And it really shouldn't be calling wait each time around the loop.

The normal Lwt way to do that would be to take Lwt_switch argument and abort the loop when the switch is turned off.
A switch can only be turned off once and so you can't miss the event. Also, the switch argument can be optional in the common case where you never want to stop.

mirage-capnp

This doesn't use conduit at the moment. It provides its own dynamic flow type over mirage-flow:

https://github.com/mirage/capnp-rpc/blob/53a38de36d02618eb87e99703f6338dbadcf5845/capnp-rpc-net/endpoint.ml#L10

Combined with capnproto/capnp-ocaml#71, the conduit API might make things more efficient by allowing us to read into a pre-existing buffer, avoiding a copy.

However, the current recv behaviour is no good - it needs to return as soon as it has some data, not wait for the buffer to be filled.

0install

I was overriding a lot of stuff in cohttp/conduit to ensure that certificate authentication was configured correctly. I need to check the new system, but it looks like authentication is configured by default now, which is great!

[hannesm]

On the plus side, from the code it looks like certificate checks now default to "on", which is a major improvement!

Yes indeed (in the lwt-unix case at least). Based on @emillon work we now have https://github.com/mirage/ca-certs (no support for windows yet, though) that reads the default trust anchors on your Unix system (Linux/BSD/macOS) and uses the current time for certificate validation. If you run into trouble with the ca-certs package, please report an issue, and we'll make sure that it works on your favourite Linux distribution. (PRs for (a) windows support and (b) macOS via system API instead of calling the security binary are welcome as well.)

For MirageOS (i.e. no host system trust anchors), a similar library https://github.com/mirage/ca-certs-nss is available which imports the trust anchors from Firefox / Netscape Security Services.

I announced these packages at https://discuss.ocaml.org/t/ann-ca-certs-and-ca-certs-nss/6804

[dinosaure]

Hi, thanks for this huge feedback 👍, I will try to explains all of these points and explain possible solutions

There is no direct way to pass a port to resolve. The example at https://mirage.github.io/ocaml-conduit/conduit-lwt/Conduit_lwt/index.html#resolution defines an http_resolver that always fixes port as 80.

I think you point the biggest problem about the new API of Conduit. At this level, I was thinking about a possible abstract way to handle any protocols. For me, the idea of a port is widely required by protocols but it's not the systematic case. For example, an UNIX domain socket does not need such information, or VCHAN.

So I took the most common and required information which can be used by any protocols. A domain-name (and therefor an IP) seems, for me, a well unique identifier to recognize, from others, an endpoint. Others information, such as the port, are a complement of Endpoint.t.

I think this assumption seems unusual for most of real cases but it fits well for a MirageOS (in my view) which can use some others protocols than TCP/IP. I tried to explain that here but a more clear explanation is: we should separate what comes from a context and what we strictly need. By this way, a port, a TLS certificate, an private SSH key, etc. is a decoration around the Endpoint.t and they appear, as you said^[1], when we want to construct the resolvers.

I suspect that users will either want to supply the configuration directly (using connect instead of resolve)

See Conduit.connect.

or connect using a URI rather than a domain name. Perhaps the endpoint and resolver stuff should be moved to a separate library, to simplify the conduit API?

My idea is to make an extra library (which can be in the Conduit distribution) to handle the Uri.t case. We currently are able to reproduce the old behavior of Conduit 2.0 with this current implementation (as I did in CoHTTP but in a more general way). However, we will repeat the same error as before. I have some strong and bad feelings about what we did before to finally provide an user-friendly way to initiate a connection (val connect : ctx:ctx -> Uri.t -> flow) - and it's not the right direction even if some people will happy to "just" launch a connection.

I think we missed several extra cases (such as Git) and focus on only what CoHTTP expects - then, we tried to enforce as much as we can others protocols like Git/Websocket/VCHAN to this layout without something homogeneous and consistent. Conduit 3.0 breaks all of these things and even if we can say that we have some kind of regressions about usability (and factually it's true), I think we provide a more interesting way which unlocked several possibilities (such as the possibility to use your own Tls.Config.t at least or a possible way to destruct your Conduit.flow to the underlying socket - and finally, be able to call getpeername).

From my experiments with ocaml-git and ocaml-cohttp (which have different concrete type to express an endpoint), a pattern appears to be able to initiate a connection

type something

val endpoint_and_resolvers_from_something : something -> (Conduit.Endpoint.t * Conduit.resolvers)

let connect : ?resolvers:Conduit.resolvers -> something -> result = fun ?(resolvers= Conduit.empty) something ->
  let edn, resolvers' = endpoint_and_resolvers_from_something something in
  let resolvers = Conduit.merge (* like Map.merge to keep user-defined resolvers *) resolvers resolvers in
  Conduit.resolve ~resolvers edn >>? fun flow ->

I found this a bit frustrating because it starts by assuming the existence of a conduit-aware getline function, which is never defined. Therefore it is not possible for the user to run the example.

Yes, you are completely true and we should rework on the tutorial - I was thinking that it was not on my responsibility (as Conduit) to provide getline... A gist exists which is what the tutorial wants to explain: gist but it requires, at least, ke as a ring-buffer to implement getline.

(and when I tried it, it returned that it had read 0 bytes of data for some reason)

This reason is a bit complex (and should discuss about that in an other issue) but it's currently hard to fix the semantic of recv according to all others projects which use Conduit. This issue comes with some others big issues which can be condensed in one problematic: a POSIX-compliant interface of Mirage_flow.S.

ocaml-conduit/src/mirage/conduit_mirage_flow.mli

Lines 1 to 11 in 1ebbd61

	(** An implementation of [conduit-lwt] according the interface [Mirage_flow.S].
	This module is deprecated when the current implementation of [read] has
	another behaviour:
	[conduit] provides:
	{[ val read : flow -> Cstruct.t -> (int or_eoi, error) result Lwt.t ]}
	where [mirage-flow] expects:
	{[ val read : flow -> (Cstruct.t or_eoi, error) result Lwt.t ]}

Conduit.S.scheduler defines type scheduler as abstract (https://mirage.github.io/ocaml-conduit/conduit/Conduit/module-type-S/index.html#type-scheduler). This doesn't appear to be used anywhere else in the API. It can probably be removed.

So you just highlight the most strangest type trick of Conduit 🎉 ! So you are right that we should remove it from the API - however, we really need it internally from some obscure reasons.

type error is defined as either Msg or Not_found. It's not clear what Not_found means in this context. In particular, recv, send and close all say they can return this.

I agree with you that Not_found should be better - at least, I think we should have Not_found of Endpoint.t to say that according to the given resolvers, we did not find anything to initiate a connection to this endpoint.

It would be good to specify the behaviour of these functions more precisely.

For all of your points, I would like to say:

1. I don't have strong opinion on the behavior of recv/send/close - at least, I would like to have a consensus on these things
2. Conduit can not assert that an implementation (such as Conduit_mirage_tcp or Conduit_lwt.TCP) respects these behaviors. Currently, the behavior of Conduit.{recv,send,close} depends on the underlying protocol implementation used (and I think the only way to solve this issue is to find a consensus on that, at least, into the MirageOS ecosystem)

Then, you have the possibility to prove which implementation you use but we can not go further.

What does register ~protocol do? Where is it registered?

I think that we did not try to detail what does register ~protocol do because we need to explain some internals stuffs or type witnesses which is not so easy. So the registration of a protocol implementation allows Conduit to re-extract the implementation from anywhere - so we register the "module" into a global Hashtbl.t. It's pretty-much the mechanism of Conduit 2.0 but it's less obscure and cryptic I believe.

I got a bit lost with the protocols and services stuff. I guess this is so that I can pass e.g. TCP or TLS to something and have it connect/accept by itself. But since it will have to provide the port and, for TLS, security information, it seems that this can't be quite generic.

It can not be by essence. I mean Conduit 2.0 shows to us the limit to try to homogenize any protocols into one and unique type edn and it came with several issues:

• the impossibility to extend this type without an other release of Conduit
• default (needed) arguments which are specific to a protocol (such as Tls.Config.client)

The goal of Conduit 3.0 is to provide a generic way to choose the protocol implementation - which is a bit different to: a generic value to represent a protocol implementation.

seems to boil down to "Give me an argument and a function and I'll call it for you".

You are true but several questions come then about "the way" to implement the main server loop:

• the configuration needed to initiate the "socket"
• the scheduler (how to yield)
• the process to pack to an abstract value the produced "socket" (see Conduit_{lwt,async}.serve does not need the protocol anymore #337 and Remove protocol argument to Service.register #338)

A choice was made to be less magic about service - we don't have a dispatch (such as Conduit.resolve) - because the end-user should be aware about the way to initiate a service (which port, which TLS certificate, which special information) and we should not hide some of these parameters to him (in the way to be more user-friendly). Finally, the service part of Conduit 3.0.0 ensures only a common interface.

It would be nice to have a construction function for Conduit_lwt.TCP.configuration. Then it could supply defaults for capacity and the bind address, and not break if we add new fields later.

Agree with that 👍.

Not sure why there's an extra () on Server.create

It comes from a special request for Irmin: here

It appears that create ignores the backlog unless the service happens to be TCP.

Not sure to understand what you mean.

Hmm, there is a Conduit_lwt.serve. But it returns a Lwt_condition.t to stop it. That's racy - if you broadcast on stop before the loop calls wait then it will miss the event! And it really shouldn't be calling wait each time around the loop.

The normal Lwt way to do that would be to take Lwt_switch argument and abort the loop when the switch is turned off.
A switch can only be turned off once and so you can't miss the event. Also, the switch argument can be optional in the common case where you never want to stop.

I trust on you and if you can propose a PR about that, I will happy to review and merge it 👍 - note that this part is outside the core of Conduit (you should take a look on Conduit_{async,lwt}.serve).

Combined with capnproto/capnp-ocaml#71, the conduit API might make things more efficient by allowing us to read into a pre-existing buffer, avoiding a copy.

I think, the question is more about the Mirage_flow.S interface than Conduit. Again, this part is about the implementation of protocols which is not the part of the core - but, yes, we really should improve it!

[dinosaure]

State whether recv does a single read or tries to read its buffer (should be single IMHO, although TCP is perhaps not doing this at the moment).

#358 fix this issue and does a single and unique read now. But what I said before still is true.

Likewise, define what send does (one call or many). mirage-flow says "There is no indication when the buffer has actually been read and, therefore, it must not be reused". It would be good to clarify whether or not conduit has this restriction.

Again, #358 will fix this issue too. We will follow exactly what Lwt_unix.write does.

close says Subsequent calls to recv will return Ok `End_of_flow. How do I shut down just the sending side and then wait for the response?

Unfortunately, it seems that you require something more complex than recv/send/close. In that situation, you should not use Conduit, or you must extract the Conduit.flow to the Lwt_unix.file_descr to be able to use some specific calls such as Lwt_unix.shutdown. For my perspective, this situation does not concern Conduit:

1. the double-close or the impossibility to use shutdown was already an issue of Conduit (2 or 3)
2. we are constrained by the Conduit.PROTOCOL signature which can not expose protocol-specific operations - such as shutdown (or any others protocols must provide the same - with the same behavior)

But the idea of Conduit 3 (despite Conduit 2) is the ability to extract the Conduit.flow to what it is really - and, in some case, a Lwt_unix.file_desr. But it implies a certain deal to the developer. If you are in the situation where you want to implement a higher protocol such as SMTP and you want to use some specific operations (such as getsockopt/shutdown/sendfile/etc.), you assume to be protocol-dependent (and target-dependent).

In that case, Conduit is not the solution because you don't want to be abstract over the protocol. An abstraction over the protocol implies, in any way, some limitations, whether by functor or by Conduit. Conduit is like an injection of a specific implementation to something else without functors. But it still is, in terms of OCaml, an other kind of a functor. So it's not magic and when you use Conduit, we need to accept the deal to restrict your use of your protocols to a simple and common signature (e.g. Conduit.S.PROTOCOL).

[talex5]

I've made some benchmarks of various APIs (mirage-flow, conduit 3, objects) here: https://github.com/talex5/flow-tests