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

Exception when client closes connection. Is this the expected behavior for async? #117

Open
eugecm opened this issue Apr 13, 2020 · 2 comments

Comments

@eugecm
Copy link

eugecm commented Apr 13, 2020

Please bear with me as I'm new to OCaml and the whole ecosystem. Currently using websocket_async.

The current implementation seems to raise an exception whenever a client disconnects. This can be easily reproduced with wscat-async on master as follows:

A server:

> ./wscat.exe -s ws://localhost:9999

Client

> ./wscat.exe ws://localhost:9999/ws

Then press Control-C on the client.

The output of the server:

2020-04-13 22:02:12.066187+01:00 Error ("Exception raised to [Monitor.try_with] that already returned.""This error was captured by a default handler in [Async.Log]."(exn(monitor.ml.Error End_of_file("Raised at file \"core/websocket.ml\", line 268, characters 14-31""Called from file \"src/deferred0.ml\", line 54, characters 64-69""Called from file \"src/job_queue.ml\", line 167, characters 6-47""Caught by monitor Monitor.protect"))))

Which points to

| None -> raise End_of_file

I guess it can make sense to raise an exception there, however (and this is what caught my attention), the async implementation also propagates the exception back to the caller as seen in:

Monitor.protect
~finally:begin fun () ->
Pipe.close ws_to_app ;
Pipe.close_read app_to_ws ;
Deferred.unit
end begin fun () ->
Deferred.any
[transfer_end;
loop ();
Pipe.closed ws_to_app;
Pipe.closed app_to_ws]
end >>= Deferred.Or_error.return
(Monitor.protect propagates)

The server function returns an 'a Async.Deferred.Or_error.t but according to the documentation for Async_kernel__.Deferred_or_error the idiom is to never raise:

The mental model for a function returning an 'a Deferred.Or_error.t is that the function never raises. All error cases are caught and expressed as an Error _ result. This module preserves that property.

Should protect be changed for a try_with_or_error?

@SeedyROM
Copy link

No answer in 3 years huh?

@vbmithr
Copy link
Owner

vbmithr commented Jul 17, 2023

I don't have time to maintain it anymore. Maybe check out https://github.com/janestreet/async_websocket for websocket server with async.

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

No branches or pull requests

3 participants