fedora-infra / mirrormanager2

Rewrite of the MirrorManager application in Flask and SQLAlchemy
https://mirrormanager.fedoraproject.org
GNU General Public License v2.0
63 stars 46 forks source link

crawler: correctly handle keep-alive for HTTPS #245

Closed adrianreber closed 6 years ago

adrianreber commented 6 years ago

PR #240 added support to actually scan HTTPS URLs with HTTPS instead of always using HTTP for HTTPS (really!). The changes from PR #240, however, let to errors in the keep-alive handling of MirrorManager.

MirrorManager tries to keep connections open if the remote side supports keep-alive and reuses that connection on subsequent requests. MirrorManager, however, does not actively track if the keep-alive connection has been closed by the remote side. If a request fails MirrorManager does single retry before returning a failure.

For HTTP connections the try/except block seems to be different than for HTTPS connections.

This adds the code which fails during remotely closed keep-alive HTTPS connections to the try/except block which seemed to be enough for HTTP connections.

See: https://pagure.io/fedora-infrastructure/issue/6778

Thanks to Fabrice Bellet for providing an easy reproducer to actually understand what was happening here.

Signed-off-by: Adrian Reber adrian@lisas.de

adrianreber commented 6 years ago

@bowlofeggs Thanks for the detailed review and I agree that implementing your comments would be nice. As I just recently understood that part of MirrorManager and as I was mainly looking to fix the currently existing bug, I will now merge this PR as it is.