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

Should runTCPServer / runTCPServerWithHandle return Void? #48

Open
bitemyapp opened this issue Jun 15, 2018 · 4 comments
Open

Should runTCPServer / runTCPServerWithHandle return Void? #48

bitemyapp opened this issue Jun 15, 2018 · 4 comments

Comments

@bitemyapp
Copy link

runTCPServerWithHandle :: ServerSettings -> ConnectionHandle -> IO a
runTCPServerWithHandle (ServerSettings port host msocket afterBind needLocalAddr _) handle =
    case msocket of
        Nothing -> E.bracket (bindPortTCP port host) NS.close inner
        Just lsocket -> inner lsocket
  where
    inner lsocket = afterBind lsocket >> forever (serve lsocket)
    serve lsocket = E.bracketOnError
        (acceptSafe lsocket)
        (\(socket, _) -> NS.close socket)
        $ \(socket, addr) -> do
            mlocal <- if needLocalAddr
                        then fmap Just $ NS.getSocketName socket
                        else return Nothing
            _ <- E.mask $ \restore -> forkIO
               $ restore (handle socket addr mlocal)
                    `E.finally` NS.close socket
            return ()

Quite possible I am wrong but IO a seems misleading to me.

/cc @mgsloan

@mgsloan
Copy link

mgsloan commented Jun 15, 2018

In favor of this 👍 - see commercialhaskell/rio#28

@snoyberg
Copy link
Member

And from that issue, you can tell that I'm -1. I'd be happier with void instead of Void. I've tried hard to avoid breaking changes in this library, and don't see this as the time to make them.

@bitemyapp
Copy link
Author

bitemyapp commented Jun 17, 2018

Even just the type variable name and a comment would help. I'd like to rip the bandaid off on this in the future if there's an opening.

@snoyberg
Copy link
Member

Here's my article on the topic: https://www.fpcomplete.com/blog/2017/07/to-void-or-to-void. I don't think there should be a bandaid ripping off moment. I think void is the correct solution in positive position, and Void in negative position. I've heard the counterarguments, but they don't hold with me. I don't find being forced to sprinkle absurd through a code base adds meaningful type safety, and does add burden to a user of a library who probably just wanted the type to unify to (). So PR welcome for the change to void with more docs, but I'm not on board with a plan for a future move to Void.

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