http-client-tls: Also catch TLSError exceptions

[erikd]

No description provided.

[erikd]

Wow, still getting:

https-client: Error_Packet "partial packet: expecting 16408 bytes, got: 2887"

even with this patch.

[erikd]

Just re-checked my test setup and I am very definitely getting this exception even with this patch.

However, this patch is useful and should probably be applied regardless.

[creichert]

I'm definitely getting exceptions related to the PR. What needs to be done to get it merged?

@erikd I agree it should be merged either way. Do you have a URL which reproduces the Error_Packet exception? I'd love to help if possible

[erikd]

@creichert The only way I was able to reproduce this was by hammering an Amazon s3 bucket (100+ threads downloading parts of a 64+ gigabyte file) until the AWS rate limiter kicked in an started killing connections. Unfortunately this means that there is nothing as simple as a URL that reproduces the error I was chasing.

I also tried writing a simple warp-tls client to reproduce the AWS rate limiting behavior but that yielded nothing.

I still think using exceptions for error handling is a mistake.

[creichert]

@erikd thanks! too bad.

I do think that even if http-client wasn't using exceptions (and using something like Either) this bug would still potentially slip through because the underlying library is creating a new concrete data type for each exception, as opposed to rolling them into a sum type.

This may not help much, but here is a way to repro a Error_Protocol that this PR should fix by catching and wrapping:

#!/usr/bin/env stack
-- stack -v runghc --package connection --package http-client --package http-client-tls
{-# LANGUAGE OverloadedStrings #-}

import Prelude

import Network.HTTP.Client.TLS
import Network.HTTP.Client

main :: IO ()
main = do
    let (Just req) = parseUrlThrow "https://paste.debian.net"
    man <- newManager tlsManagerSettings
    _res <- httpLbs req man
    print "success"

tls-error-repro.hs: Error_Protocol ("bad SignatureRSA for ecdhparams",True,HandshakeFailure)

---

Edit To clarify, you have:

-- tls
instance Exception TLSError
instance Exception TLSException

-- connection
instance E.Exception LineTooLong
instance E.Exception HostNotResolved
instance E.Exception HostCannotConnect

So if a new one is added, it may be difficult to remember to check that, even if cabal version of the lib is incremented properly.

[erikd]

I'm not blaming http-client for using exceptions, I am blaming the underlying libraries. I think that if the underlying libraries hadn't used exceptions, then http-client wouldn't have had to handle them.

[erikd]

At work we have a large code base that always tries to catch exceptions as close as possible to their source and turn them into Eithers. It works really well until we have to use a library that allows exceptions to escape.

[snoyberg]

Good call @creichert, and thanks @erikd. Released to Hackage.

[ocheron]

I know this is not satisfactory, but see why Error_Protocol is thrown by tls instead of being wrapped inside HandshakeFailed, and will make a PR.

This for a different reason than haskell-tls/hs-tls#220. Exceptions occuring at a renegotiation handshake are simply not wrapped.

@creichert Your example has been very helpful, but must be using tls-1.3.9 or even earlier.