search

ocsigen / lwt

OCaml promises and concurrent I/O
https://ocsigen.org/lwt
MIT License
706 stars 172 forks source link

Error code from getaddrinfo is lost #835

Open Lupus opened 3 years ago

Lupus commented 3 years ago

It seems that in case of getaddrinfo failure, the error is... lost? https://github.com/ocsigen/lwt/blob/1157930612348286fcb5b675b3309f04e0e5a2b8/src/unix/unix_c/unix_getaddrinfo_job.c#L74 There is no "else" branch here. I get empty list from Lwt_unix.getaddrinfo and I'm trying to figure out why that happens.

aantron commented 3 years ago

That does seem wrong @Lupus. I checked the standard Unix, and it looks like the Lwt code is directly based on it: https://github.com/ocaml/ocaml/blob/d14b3ec19f44fc4ac88c1fa4b7c97e201c689b96/otherlibs/unix/getaddrinfo.c#L117. It also doesn't have an else branch. Could you double-check using Unix?

I think we should add error handling. I also suggest to report the issue to the OCaml repo if you can confirm it, so we can get the benefit of any extra discussion from the OCaml team, and/or fix the (likely) bug there, as well.

aantron commented 3 years ago

Well, one of the most immediate issues is that the EAI_* error codes are not included in type Unix.error.

aantron commented 3 years ago

They do, however, appear in the POSIX man pages for the functions, e.g. https://man7.org/linux/man-pages/man3/freeaddrinfo.3p.html#ERRORS. So this could be an argument for adding them (eventually?). They are also present in libuv (http://docs.libuv.org/en/v1.x/errors.html#error-constants), which is part of why I forgot they are missing from Unix, at first.

aantron commented 3 years ago

In the near term, we could map all of them to Unix.EUNKNOWNERR from Unix.error.

Lupus commented 3 years ago

Could you double-check using Unix?

I don't have a minimal reproducible test case for that, I only see that sometimes I get exceptions from List.hd in production logs :) So I can't really check it with bare Unix.

In the near term, we could map all of them to Unix.EUNKNOWNERR from Unix.error

Probably we could get stringified message for EAI_* code and add it into one of string components of exception Unix_error of error * string * string? And raise it directly.