fdopen / uwt

libuv bindings for OCaml
https://fdopen.github.io/uwt/
MIT License
55 stars 7 forks source link

Suggestion for coping with Unix_error vs Uwt_error #2

Closed djs55 closed 7 years ago

djs55 commented 8 years ago

This library is working well for me so far -- thank you for writing it!

I made an error porting some code from Lwt_unix to Uwt where I caught Unix_error rather than Uwt_error in my exception handler.. see the patch in [docker/vpnkit#94]

Assuming Uwt_error needs to be different from Unix_error, perhaps it would be safer to use a (a, Uwt.error) Result.result to force people to match on the correct constructors? Especially people like me who are porting old code to this library, or who have Unix_error in their muscle memory :)

fdopen commented 8 years ago

I see, it's the usual problem with exceptions. I've introduced Uwt_error, because there are error codes, that are missing inside Unix, e.g. ECANCELED. ECANCELED is used internally by libuv at several locations (→ libuv will report ECANCELED although no system call reported ECANCELD).

However, switching to Result.result would be a huge interface change. Another possibility would be to use Unix.Unix_error and live with constructs like:

Lwt.catch ( fun () -> ... ) (function
| Unix.Unix_error(Unix.EUNKNOWNERR(ec),s1,s2) 
    when ec = (Uwt.Int_result.ecanceled :> int) -> ...
| x -> Lwt.fail x )

I'm not yet sure about it.

dsheets commented 8 years ago

If you are interested in Unix errnos from Linux, FreeBSD, and OS X that are available without any unix dependency, you might like dsheets/ocaml-unix-errno. Maybe it's right for this use case, maybe it's not. If you try it and find that it has some subtle Windows incompatibility or Unix dependence, please let me know and I will fix it ASAP.

fdopen commented 8 years ago

The problem are not missing errnos (except for ECANCELED). Libuv error codes (represented as Uwt.error or Uwt.Int_result.t) are an union of unix errno codes, libuv's own error codes, and other custom error codes (e.g. as defined in netdb.h).

I will change the interface of functions, that can return non unix errno codes, to ('a, Uwt.error) result Lwt.t (there aren't many and most already return a result type). All other functions will Lwt.fail with Unix_error. Changing all functions to result would require too many changes in other code for the moment.

(I had a quick look at ocaml-unix-errno. What's the intention behind these #ifdef's: https://github.com/dsheets/ocaml-unix-errno/blob/3a9b894c8d43c88d977a344ec499c37158be9993/unix/unix_errno_util.c#L23 It won't compile on netbsd, openbsd, or any other OS that you didn't consider.)

dsheets commented 8 years ago

Indeed. Initially, those were put in place to explore the disparities between errnos on different operating systems but they should likely be changed to simple #ifdef ERRNO now.