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

The getNameInfo API is a little strange #416

Open
peti opened this issue Jun 28, 2019 · 6 comments
Open

The getNameInfo API is a little strange #416

peti opened this issue Jun 28, 2019 · 6 comments

Comments

@peti
Copy link

peti commented Jun 28, 2019

getNameInfo has the ability to report a name resolution error by means of returning Nothing. When the NI_NAMEREQD flag is not set, resolution will never fail because the function simply returns the IP address we were trying to look up. When NI_NAMEREQD is set, however, the function is supposed to fail when that address can't be resolved. Curiously enough, that failure is communicated through an IOException instead of simply returning Nothing! This is really surprising to me and I find myself writing code like

getNameInfo [NI_NAMEREQD] True False localAddr `catchIO` \_ -> return (Nothing,Nothing)

most of the time.

Is there any particular reason why an IOException is used to communicate a perfectly legitimate result of that function call?

@peti
Copy link
Author

peti commented Jun 28, 2019

To make matters even more complicated, there are use cases for getNameInfo that cannot fail, i.e. when NI_NUMERICHOST is requested. Yet, the API's use of Maybe suggests that this operation can fail. Furthermore, that operation does not require the IO monad because it has no side-effects.

I suppose you'd really want a family of functions that express different operations with an API that's appropriate. inet_ntoa et al comes to mind here ...

@kazu-yamamoto
Copy link
Collaborator

@eborden We should also consider this when we introduced a new error mechanism.

@kazu-yamamoto
Copy link
Collaborator

@peti The current maintainers of network are not the original author of getNameInfo. So, we don't know this kind of intention in detail. Sorry.

@eborden
Copy link
Collaborator

eborden commented Jul 1, 2019

#151 is the primary issue for rethinking error handling. Mentioning it in this issue will make it visible there and more likely to be considered when addressing error handling in general.

@peti, I agree it is surprising. I don't see anything in git blame to give a deeper reason, so thank you for documenting this as an issue.

Is there a reason to expand failure modes beyond Maybe to an Either with more context or should the Maybe be removed and the throwing behavior be maintained (with better exceptions)?

@peti
Copy link
Author

peti commented Jul 1, 2019

Is there a reason to expand failure modes beyond Maybe to an Either with more context?

Yes, absolutely. The getnameinfo() function from C provides plenty of useful information via its return code that's currently hidden from users. According to the man page, we have:

EAI_AGAIN
   The name could not be resolved at this time. Try
   again later.

EAI_BADFLAGS
   The flags argument has an invalid value.

EAI_FAIL
   A nonrecoverable error occurred.

EAI_FAMILY
   The address family was not recognized, or the address
   length was invalid for the specified family.

EAI_MEMORY
   Out of memory.

EAI_NONAME
   The name does not resolve for the supplied arguments.
   NI_NAMEREQD is set and the host's name cannot be
   located, or neither hostname nor service name were
   requested.

EAI_OVERFLOW
   The buffer pointed to by host or serv was too small.

EAI_SYSTEM
   A system error occurred. The error code can be found
   in errno.

Some of those (like EAI_OVERFLOW) are probably irrelevant for Haskell code and should not be exposed to the user, but I'd certainly be able to distinguish between "the name does not resolve" and "cannot reach the name resolver" type of errors.

Or should the Maybe be removed and the throwing behavior be maintained (with better exceptions)?

Personally, I prefer Either ErrorType any day. Exceptions should be thrown only, IMHO, if we expect that the user's code will want stack unwinding because that particular error can most probably not be dealt with at the site that calls getNameInfo. The error EAI_MEMORY, for example, should probably be an exception, because it signifies some greater problem that goes beyond the scope of "name resolving" and probably cannot be handled in code that tries to resolve a name. The error EAI_AGAIN, however, is a totally legitimate result of getNameInfo and throwing it as an exception feels like counter-productive (and unnecessarily expensive).

@kazu-yamamoto
Copy link
Collaborator

@eborden Thank you for explaining. I understand the motivation for the new error handing. That is, we should expose errors of system calls. But #151 also says that errors should be extendable. What does this mean?

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