janestreet / async_unix

Jane Street Capital's asynchronous execution library (unix)
MIT License
33 stars 21 forks source link

Unix_syscalls.readdir returns a string and raises on end-of-dir #13

Closed dsheets closed 7 years ago

dsheets commented 8 years ago

True to the Unix.readdir interface and readdir(3) before it, Unix_syscalls.readdir returns but a string and raises End_of_file when the directory handle is exhausted. To use it correctly, the programmer must write something like:

try_with ~extract_exn:true (fun () -> Unix_syscalls.readdir dh)
>>= function
| Ok dirent -> next (dirent::listing)
| Error End_of_file -> return listing
| Error exn -> raise exn

This is particularly bad because ~extract_exn:true must be passed to try_with or the end-of-directory condition is effectively un-catchable as the wrapped exception that is raised is not exposed in an interface (as far as I could tell). This seems like fairly poor ergonomics that should either be corrected or very clearly documented as legacy.

A final minor issue is the uncertainty about the | Error exn -> raise exn clause which, from an LWT programmer's point of view, looks wrong because the exception is not injected back into the monad and instead a try .. catch is necessary inside bind or similar to make sure that these truly exceptional cases are handled correctly and passed upward mostly un-modified. Of course, some metadata is probably lost here but I'll let you be the judge of how horrible that is.

ghost commented 8 years ago

I agree that this behavior is bad. I did some grepping in opam and it looks like changing Unix.readdir to return an option is going to break a lot of things. As a first step we'll add a second function and deprecate readdir

bmillwood commented 7 years ago

There is a Unix.readdir_opt now.