Skip to content

WIP: Transition from Lwt to Eio #269

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

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from
Draft

WIP: Transition from Lwt to Eio #269

wants to merge 23 commits into from

Conversation

Julow
Copy link
Contributor

@Julow Julow commented Jun 19, 2025

On top of #267 #268 #266

The first commit are the changes automatically made by this tool: https://github.com/Julow/lwt-to-direct-style with the following options:

lwt-to-direct-style --migrate --eio-sw-as-fiber-var=Ocsigen_lib.current_switch --eio-env-as-fiber-var=Ocsigen_lib.env

The second commit makes all the manual changes required to make the server works. It does many things and I plan to split it up into smaller commits with better messages.

The tests run but I've not tested this in a real application yet. There are no compatible Eliom version yet.

Julow added 11 commits June 12, 2025 14:35
`of_cohttp_body` is added as it is needed by `Ocsigen_multipart`. This
can be ported to other representations of cohttp's Body later if needed.
This upgrade is needed to be able to target `cohttp-eio` which was added
in version 6.0.0.

Cohttp doesn't carry the content encoding in the Response type so some
code is added to add the `transfer-encoding` header.
This is equivalent to `Cohttp_lwt_unix.Server.respond_file` but is
written directly against `Ocsigen_response.Body.t`, which is easier to
translate to direct-style.
This upgrade is needed to be able to target `cohttp-eio` which was added
in version 6.0.0.

Cohttp doesn't carry the content encoding in the Response type so some
code is added to add the `transfer-encoding` header.
Cohttp_eio substitutes Cohttp_lwt_unix.

Uncaught exceptions are now logged with `Cohttp_eio`'s `~on_error`
callback.

The active switch and the environment are passed using Fiber variables.

Ocsigen_stream: Read loop adapted to Eio

Ocsigen_response: Adapted to Eio.File and use unbuffered Eio.Flow read

Ocsigen_response: Cohttp_eio doesn't expose the internal Response module.

Ocsigen_server: Server no longer listen on IPv6 when started with `` ~ports:[ `All ] ``.

Revproxy: Implement `get_inet_addr` with Eio
Revproxy: HTTPS is not supported out of the box in Cohttp_eio

Async exceptions are now handled with a result type:
`Fiber.fork_promise` returns a `('a, exn) result Promise.t` and no
longer use a callback for exceptions.

Add dependency on magic-mime. It was a transitive dependency through cohttp-lwt-unix.

Command pipe: Fork daemon

Use Eio.Exn.pp instead of Printexc.to_string in log messages.
Eio exceptions are printed in a better way with regard to the margin.
Eio doesn't allow filesystem operations to escape the current directory.
This is a safety feature but might break applications.
The tests are changed to avoid the symlinks pointing outside of the
current directory that Dune creates.
@Julow
Copy link
Contributor Author

Julow commented Jun 19, 2025

Important work remain to be done:

  • TLS is not implemented by cohttp-eio. It was previously done by conduit, which doesn't have a eio version yet.

  • Eio's Switch mechanism should be used to make sure resources are scoped to a request.

  • Staticmod can't access files outside of the current directory anymore. This means that it won't follow symlinks to files outside of the current directory. Whether this breaks applications should be checked.

  • The server no longer listens to IPv6 when started with ~ports:[ `All ].

  • The code for querying the environment and current switch could be improved.

Julow added 3 commits June 19, 2025 19:16
This sets lower bounds for eio and ensure compatibility with a wider
range of versions.
balat and others added 9 commits June 20, 2025 14:28
This list of dependency is used to generate static.ml, which needs the
list to be uptodate to be able to load extensions correctly when run
with `ocsigenserver -c conf_file`.
This makes `Ocsigen_request.t` lighter and remove a source of
exceptions.

Value of type `Unix.sockaddr` were constructed for purposes other than
performing network calls and can easily be replaced by a string.

Accesscontrol is the only user of parsed IP addresses, which doesn't
justifies the weight added to the request record.
This removes a source of exception for every requests.
Curl sets a User-agent value that changes with each version and causes
unwanted diffs in log outputs.
The usual setup has served file in a different location (for example,
/var) than where the server is run from (usually the home directory).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants