simpleHTTP: Catch IOException from openStream and turn it into Result

[oherrala]

openStream can fail for example when connecting to host fails. Catch
this and return error as Result with message from IOException instead.

While there, remove unnecessary normalizeRequest since it's called in
simpleHTTP_.

Fixes #92

[hsenag]

Hi, sorry I haven't merged this yet, I've been rather busy. The main reservation I have is that I'm not sure it makes sense to convert all exceptions into a 'Result', and if we don't convert all exceptions I'd like to have a clear principle of which ones should be converted.

For example I don't think something like 'ThreadKilledException' should be converted into a 'Result' (just to be clear, your patch doesn't do that as it's an 'AsyncException').

That said, I don't have a clear idea of the right principle and perhaps all 'IOException's are fine to catch. What I roughly have in mind is that locally-generated errors - e.g. issues with the program or the code - should be real exceptions, whereas remotely-generated errors (problems with the server) should be in 'Result'. I'm not sure where things like networking problems and your other pull request for 'https' behaviour fit into this.

It would also be helpful to have tests for this change, e.g. that check that #92 is dealt with, and cover any other desired error cases that should be caught.

[oherrala]

I'm not sure if I have enough experience for arguing, but lets try. I agree about AsyncException. Programming errors and exceptions not related to what we are doing (IO?) should also be exceptions as you say.

But the library user's experience for simpleHTTP should be a total function (or as close as possible). If it's not, the user of the library is left with handling the Either as well as catching the exceptions to be able to write robust code. And it's difficult to reason about exceptions.

However, ConnError type might probably need looking into so everything is not tucked in ErrorMisc but maybe something like ErrorIO for errors from IOException. This is probably worth it's own issue/pull request.

Test case for #92 might be a bit tricky. My shot at it below. I can commit this if it's acceptable solution.

The problem is that we need to try to connect TCP socket. And that requires host and port that are not in use. My solution is to create listening TCP socket with backlog of length zero to make connection fail. This however might not be portable, but Linux, OpenBSD, FreeBSD and Mac OS X listen(2) state:

The backlog parameter defines the maximum length the queue of pending connections may grow to. If a connection request arrives with the queue full the client may receive an error with an indication of ECONNREFUSED, or, if the underlying protocol supports retransmission, the request may be ignored so that retries may succeed.

Test function:

connectionRefused :: Assertion
connectionRefused = do
  response <- withFreePort $ \port -> simpleHTTP (getRequest $ "http://127.0.0.1:" ++ port)
  assertEqual "Connection is refused"
              (Left (ErrorMisc "connect: does not exist (Connection refused)"))
              (fmap show response)
  where
    -- Bind to TCP port and listen it with 0 backlog. This way the port is
    -- reserved and connection to it will be refused.
    withFreePort f = do
      socket <- Socket.socket Socket.AF_INET Socket.Stream Socket.defaultProtocol
      Socket.bind socket (Socket.SockAddrInet Socket.aNY_PORT Socket.iNADDR_ANY)
      Socket.listen socket 0
      (Socket.PortNum port) <- Socket.socketPort socket
      result <- f (show port)
      Socket.close socket
      return result

[hsenag]

By coincidence this issue has just come up for another http library:

https://www.reddit.com/r/haskell/comments/4oi5us/feedback_requested_how_to_deal_with_exceptions/
Maybe we can just copy what they decide.