From 7904f12d0578628b59420fea2543c0dbc1a57849 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 26 Feb 2024 11:46:08 +0100 Subject: [PATCH] net: add test of automatic retries in download_file() The existing logic from d1ddd525c in !1225 is confusing because it adds its own retry loop on top of the retry mechanism that is built into requests. So this test confirms that setting `download_file(retries=3)` actually results in more than three retries. --- fdroidserver/net.py | 14 +++++++-- tests/net.TestCase | 70 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/fdroidserver/net.py b/fdroidserver/net.py index cf01395a..49d67f2c 100644 --- a/fdroidserver/net.py +++ b/fdroidserver/net.py @@ -29,12 +29,20 @@ HEADERS = {'User-Agent': 'F-Droid'} def download_file(url, local_filename=None, dldir='tmp', retries=3, backoff_factor=0.1): + """Try hard to download the file, including retrying on failures. + + This has two retry cycles, one inside of the requests session, the + other provided by this function. The requests retry logic applies + to failed DNS lookups, socket connections and connection timeouts, + never to requests where data has made it to the server. This + handles ChunkedEncodingError during transfer in its own retry + loop. This can result in more retries than are specified in the + retries parameter. + + """ filename = urllib.parse.urlparse(url).path.split('/')[-1] if local_filename is None: local_filename = os.path.join(dldir, filename) - # Retry applies to failed DNS lookups, socket connections and connection - # timeouts, never to requests where data has made it to the server; so we - # handle ChunkedEncodingError during transfer ourselves. for i in range(retries + 1): if retries: max_retries = Retry(total=retries - i, backoff_factor=backoff_factor) diff --git a/tests/net.TestCase b/tests/net.TestCase index 48844a40..68c1c935 100755 --- a/tests/net.TestCase +++ b/tests/net.TestCase @@ -4,8 +4,13 @@ import inspect import logging import optparse import os +import random +import requests +import socket import sys import tempfile +import threading +import time import unittest from unittest.mock import MagicMock, patch @@ -20,6 +25,57 @@ from fdroidserver import common, net from pathlib import Path +class RetryServer: + """A stupid simple HTTP server that can fail to connect""" + + def __init__(self, port=None, failures=3): + self.port = port + if self.port is None: + self.port = random.randint(1024, 65535) + self.failures = failures + self.stop_event = threading.Event() + threading.Thread(target=self.run_fake_server).start() + + def stop(self): + self.stop_event.set() + + def run_fake_server(self): + server_sock = socket.socket() + server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server_sock.bind(('127.0.0.1', self.port)) + server_sock.listen(5) + server_sock.settimeout(5) + time.sleep(0.001) # wait for it to start + + while not self.stop_event.is_set(): + self.failures -= 1 + conn = None + try: + conn, address = server_sock.accept() + conn.settimeout(5) + except TimeoutError: + break + if self.failures > 0: + conn.close() + continue + conn.recv(8192) # request ignored + self.reply = b"""HTTP/1.1 200 OK + Date: Mon, 26 Feb 2024 09:00:14 GMT + Connection: close + Content-Type: text/html + + Hello World! + """ + self.reply = self.reply.replace(b' ', b'') # dedent + conn.sendall(self.reply) + conn.shutdown(socket.SHUT_RDWR) + conn.close() + + self.stop_event.wait(timeout=1) + server_sock.shutdown(socket.SHUT_RDWR) + server_sock.close() + + class NetTest(unittest.TestCase): basedir = Path(__file__).resolve().parent @@ -52,6 +108,20 @@ class NetTest(unittest.TestCase): self.assertTrue(os.path.exists(f)) self.assertEqual('tmp/com.downloader.aegis-3175421.apk', f) + def test_download_file_retries(self): + server = RetryServer() + f = net.download_file('http://localhost:%d/f.txt' % server.port) + # strip the HTTP headers and compare the reply + self.assertEqual(server.reply.split(b'\n\n')[1], Path(f).read_bytes()) + server.stop() + + def test_download_file_retries_not_forever(self): + """The retry logic should eventually exit with an error.""" + server = RetryServer(failures=5) + with self.assertRaises(requests.exceptions.ConnectionError): + net.download_file('http://localhost:%d/f.txt' % server.port) + server.stop() + if __name__ == "__main__": os.chdir(os.path.dirname(__file__))