Don't let crypton-connection do name resolution

[Yuras]

Instead use the existing resolver.

Fixes #569 and #544 (EDIT: no, it doesn't), likely fixes commercialhaskell/stack#5994

The problems is that crypton-connection tries addresses one by one, so if the fist address returned by getaddrinfo(3) is a misconfigured ipv6 address, it'll spend quite a bit of time trying to connect to ipv6 address, and only then try the next address.

Note that http-client tries connections concurrently.

I tested only getTlsConnection part and not getTlsProxyConnection since I don't actually have any proxy on hands. I let maintainer decide what to do next.

[sol]

@Yuras do you think it's possible to first do a small refactoring that extracts the bracketOnError stuff into a small helper function, and then, in a second commit, apply the semantic change at one place?

1. one commit that extracts the common pattern into a helper function and does not introduce any behavioral changes
2. a second commit that introduce the semantic change

[Yuras]

Not sure what exactly you mean, withSocket seems to be exactly the helper you are talking about... If you want to do it differently, you can just close the PR.

[sol]

Not sure what exactly you mean, withSocket seems to be exactly the helper you are talking about... If you want to do it differently, you can just close the PR.

This looks like a useful contribution, so I hope there is a better outcome than "close the PR".

That said:

• Doing the refactoring first makes it easier for me to review.
• There's also some code duplication. For example if source code comments are necessary, I would rather have them in one place.
• I'm not doing this myself. I just spent a lot of time trying to fix CI. One reason I did this is so that I can review your PR, and so that we can get it merged.

[sol]

Also, note that I was asking you a question. That means that I'm not even sure if this turns out nice. If I already know 100% that something is the right thing to do, then I will not phrase it as a question, but instead say "please do X".

[Yuras]

Looks like there is a misunderstanding here. I'm happy to refactor the PR if you explain what exactly you are expecting. But I genuinely don't understand what I should do. And since the changes are quite trivial, it's almost definitely be faster for you to do the changes than to explain it to me.

There's also some code duplication. For example if source code comments are necessary, I would rather have them in one place.

The comments are probably not necessary, I'm happy to drop them. It was just me being confused by the design of withSocket - if you compare it with e.g. System.IO.withFile, you'd expect it to close the socket on exit, but it does that only on exception.

Anyway, I used withSocket because that's how http-client and http-client-openssl are implemented.

I don't see any code duplication here. The ConnectionParams are different, the last argument of withSocket is also different in both cases. There is literally nothing to deduplicate, unless you mean refactoring withSocket itself, which is much more intrusive. But, again, I'm happy to deduplicate the code if you point me to the exact thing to deduplicate.

[sol]

Looks like there is a misunderstanding here. I'm happy to refactor the PR if you explain what exactly you are expecting.

OK, at least we sorted that out. That's the refactoring I had in mind #584.

The second commit of https://github.com/snoyberg/http-client/compare/tls-no-resolve then introduces your change.

[sol]

I'm happy to deduplicate the code if you point me to the exact thing to deduplicate.

It's less about deduplication, but more about making it easier to review, especially as I also don't have a way to test the proxy code path.

[Yuras]

Cool, thanks. Closing in favor to your branch.