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

socketPort does not handle Unix sockets #319

Closed
eborden opened this issue May 1, 2018 · 4 comments
Closed

socketPort does not handle Unix sockets #319

eborden opened this issue May 1, 2018 · 4 comments

Comments

@eborden
Copy link
Collaborator

eborden commented May 1, 2018

socketPort is a partial function. It only supports AF_INET and AF_INET6. Its type signature is not really indicative of this. IO informs that this function could throw from an exception, but partiality should be communicated with Maybe.

Originally reported by @RyanGlScott here #318 (comment)

@kazu-yamamoto
Copy link
Collaborator

kazu-yamamoto commented May 2, 2018

I'm not sure which is better:

  1. Keep this code and update the documentation
  2. Change the signature to Socket -> IO (Maybe PortNumber)

@eborden
Copy link
Collaborator Author

eborden commented May 2, 2018

@kazu-yamamoto It is a tough call. If we don't want to change the type signature then we must update the documentation.

Option 1

Pro

  • This does not cause API breakage.
  • It doesn't require pointer indirection introduced by Maybe

Con

  • We continue to have a partial function.
  • Documentation can easily get out of sync with reality.

Option 2

Pro

  • It communicates partiality within the type.
  • It leaves how to handle that partiality up to the consumer.
  • It doesn't mix true exceptions with non exceptional failure.

Con

  • It causes API breakage.
  • It requires pointer indirection, so will be less efficient.

@kazu-yamamoto
Copy link
Collaborator

I would suggest to create a new API, for instance, socketPortSafe :: Socket -> IO (Maybe PortNumber) and update the doc of socketPort.

kazu-yamamoto added a commit to kazu-yamamoto/network that referenced this issue May 25, 2018
kazu-yamamoto added a commit to kazu-yamamoto/network that referenced this issue May 25, 2018
@kazu-yamamoto
Copy link
Collaborator

socketPortSafe is implemented.

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

2 participants