Skip to content
Permalink
Browse files

ConnectionTimeoutException

Pinging @feuerbach, can you confirm that this makes the exceptions you
were seeing more meaningful?
  • Loading branch information...
snoyberg committed Apr 14, 2015
1 parent 6d88aa6 commit b86b1cdd91e56ee33150433dedb32954d2082621
@@ -1,3 +1,7 @@
## 0.4.12

* `ConnectionTimeoutException`

## 0.4.11.1

* Disable custom timeout code [#116](https://github.com/snoyberg/http-client/issues/116)
@@ -20,6 +20,7 @@ import Network.HTTP.Client.Request
import Network.HTTP.Client.Response
import Network.HTTP.Client.Cookies
import Data.Time
import Data.Maybe (fromMaybe)
import Control.Exception
import qualified Data.ByteString as S
import qualified Data.ByteString.Char8 as S8
@@ -81,7 +82,7 @@ httpRaw req0 m = do
(timeout', (connRelease, ci, isManaged)) <- getConnectionWrapper
req
(responseTimeout' req)
(failedConnectionException req)
(connectionTimeoutException (fromMaybe 0 $ responseTimeout' req) req)
(getConn req m)

-- Originally, we would only test for exceptions when sending the request,
@@ -11,6 +11,7 @@ module Network.HTTP.Client.Manager
, withManager
, getConn
, failedConnectionException
, connectionTimeoutException
, defaultManagerSettings
, rawConnectionModifySocket
, proxyFromRequest
@@ -377,6 +378,13 @@ failedConnectionException req =
FailedConnectionException host' port'
where
(_, host', port') = getConnDest req
{-# DEPRECATED failedConnectionException "Use connectionTimeoutException" #-}

connectionTimeoutException :: Int -> Request -> HttpException
connectionTimeoutException useqs req =
ConnectionTimeoutException useqs host' port'
where
(_, host', port') = getConnDest req

getConnDest :: Request -> (Bool, String, Int)
getConnDest req =
@@ -120,6 +120,10 @@ data HttpException = StatusCodeException Status ResponseHeaders CookieJar
-- and @transfer-encoding: chunked@ are used. Since 0.4.8.
--
-- Since 0.4.11 this exception isn't thrown anymore.
| ConnectionTimeoutException Int String Int
-- ^ useqs host/port
--
-- Since 0.4.12
deriving (Show, T.Typeable)
instance Exception HttpException

@@ -1,5 +1,5 @@
name: http-client
version: 0.4.11.1
version: 0.4.12
synopsis: An HTTP client engine, intended as a base layer for more user-friendly packages.
description: Hackage documentation generation is not reliable. For up to date documentation, please see: <http://www.stackage.org/package/http-client>.
homepage: https://github.com/snoyberg/http-client

2 comments on commit b86b1cd

@feuerbach

This comment has been minimized.

Copy link

replied Apr 14, 2015

Here's the issue with this patch.

Someone may be catching FailedConnectionException in order to reconnect upon reaching the timeout. In fact, that was what I considered to do.

This patch would break this code silently, in a way that even tests are unlikely to catch.

So I would prefer (in this order of preference):

  1. Documenting what FailedConnectionException means and changing its show instance. This has the benefit of not breaking any code and having most of the benefits.
  2. Actually renaming the FailedConnectionException constructor. This way any code which attempts to catch it will break, which may be annoying, but will explicitly tell the users that they should fix it.
@snoyberg

This comment has been minimized.

Copy link
Owner Author

replied Apr 15, 2015

Alright, see: 0e1a521

Please sign in to comment.
You can’t perform that action at this time.