| 1 | import io |
| 2 | import threading |
| 3 | import urllib.error |
| 4 | import unittest |
| 5 | import time |
| 6 | from unittest.mock import patch, MagicMock |
| 7 | |
| 8 | from lib import http |
| 9 | |
| 10 | |
| 11 | class Test429RetryLimit(unittest.TestCase): |
| 12 | """429 retries must be capped at max_429_retries to avoid wasting latency.""" |
| 13 | |
| 14 | @patch("lib.http.urllib.request.urlopen") |
| 15 | @patch("lib.http.time.sleep") # Don't actually sleep in tests |
| 16 | def test_429_retries_limited_to_2_by_default(self, mock_sleep, mock_urlopen): |
| 17 | """With default max_429_retries=2, should attempt 2 times then raise.""" |
| 18 | error = urllib.error.HTTPError( |
| 19 | "http://example.com", 429, "Too Many Requests", {}, None |
| 20 | ) |
| 21 | mock_urlopen.side_effect = error |
| 22 | |
| 23 | with self.assertRaises(http.HTTPError) as ctx: |
| 24 | http.request("GET", "http://example.com", retries=5) |
| 25 | |
| 26 | self.assertEqual(ctx.exception.status_code, 429) |
| 27 | # Should be called exactly 2 times (initial + 1 retry), not 5 |
| 28 | self.assertEqual(mock_urlopen.call_count, 2) |
| 29 | |
| 30 | @patch("lib.http.urllib.request.urlopen") |
| 31 | @patch("lib.http.time.sleep") |
| 32 | def test_non_429_errors_still_use_full_retries(self, mock_sleep, mock_urlopen): |
| 33 | """500 errors should still retry up to the full retries count.""" |
| 34 | error = urllib.error.HTTPError( |
| 35 | "http://example.com", 500, "Internal Server Error", {}, None |
| 36 | ) |
| 37 | mock_urlopen.side_effect = error |
| 38 | |
| 39 | with self.assertRaises(http.HTTPError): |
| 40 | http.request("GET", "http://example.com", retries=3) |
| 41 | |
| 42 | self.assertEqual(mock_urlopen.call_count, 3) |
| 43 | |
| 44 | @patch("lib.http.urllib.request.urlopen") |
| 45 | @patch("lib.http.time.sleep") |
| 46 | @patch("lib.http.time.monotonic", return_value=0.5) |
| 47 | def test_shared_deadline_stops_retry_before_backoff_crosses_it( |
| 48 | self, |
| 49 | _mock_monotonic, |
| 50 | mock_sleep, |
| 51 | mock_urlopen, |
| 52 | ): |
| 53 | mock_urlopen.side_effect = urllib.error.HTTPError( |
| 54 | "http://example.com", 500, "Internal Server Error", {}, None |
| 55 | ) |
| 56 | |
| 57 | with self.assertRaises(http.HTTPError) as caught: |
| 58 | http.request( |
| 59 | "GET", |
| 60 | "http://example.com", |
| 61 | retries=3, |
| 62 | deadline_monotonic=1.0, |
| 63 | ) |
| 64 | |
| 65 | self.assertEqual(http.health.TIMEOUT, caught.exception.outcome_state) |
| 66 | self.assertEqual(1, mock_urlopen.call_count) |
| 67 | mock_sleep.assert_not_called() |
| 68 | |
| 69 | @patch("lib.http.urllib.request.urlopen") |
| 70 | @patch("lib.http.time.monotonic", side_effect=[0.0, 0.5, 1.5]) |
| 71 | def test_shared_deadline_rejects_response_that_finishes_late( |
| 72 | self, |
| 73 | _mock_monotonic, |
| 74 | mock_urlopen, |
| 75 | ): |
| 76 | mock_urlopen.return_value = _mock_response() |
| 77 | |
| 78 | with self.assertRaises(http.DeadlineExceeded): |
| 79 | http.request( |
| 80 | "GET", |
| 81 | "http://example.com", |
| 82 | retries=1, |
| 83 | deadline_monotonic=1.0, |
| 84 | ) |
| 85 | |
| 86 | @patch("lib.http.urllib.request.urlopen") |
| 87 | def test_shared_deadline_stops_waiting_during_slow_body_read( |
| 88 | self, |
| 89 | mock_urlopen, |
| 90 | ): |
| 91 | response = _mock_response() |
| 92 | |
| 93 | def slow_read(): |
| 94 | time.sleep(0.2) |
| 95 | return b'{"ok": true}' |
| 96 | |
| 97 | response.read.side_effect = slow_read |
| 98 | mock_urlopen.return_value = response |
| 99 | started = time.monotonic() |
| 100 | |
| 101 | with self.assertRaises(http.DeadlineExceeded): |
| 102 | http.request( |
| 103 | "GET", |
| 104 | "http://example.com", |
| 105 | retries=1, |
| 106 | deadline_monotonic=started + 0.02, |
| 107 | ) |
| 108 | |
| 109 | self.assertLess(time.monotonic() - started, 0.12) |
| 110 | |
| 111 | @patch("lib.http.urllib.request.urlopen") |
| 112 | def test_shared_deadline_stops_waiting_during_error_body_read(self, mock_urlopen): |
| 113 | release = threading.Event() |
| 114 | read_started = threading.Event() |
| 115 | |
| 116 | class SlowBody(io.BytesIO): |
| 117 | def read(self, *args, **kwargs): |
| 118 | read_started.set() |
| 119 | release.wait(1) |
| 120 | return super().read(*args, **kwargs) |
| 121 | |
| 122 | mock_urlopen.side_effect = urllib.error.HTTPError( |
| 123 | "https://example.com", 500, "Server error", {}, SlowBody(b"error") |
| 124 | ) |
| 125 | started = time.monotonic() |
| 126 | try: |
| 127 | with self.assertRaises(http.DeadlineExceeded): |
| 128 | http.request( |
| 129 | "GET", "https://example.com", retries=1, |
| 130 | deadline_monotonic=started + 0.05, |
| 131 | ) |
| 132 | self.assertTrue(read_started.is_set()) |
| 133 | self.assertLess(time.monotonic() - started, 0.5) |
| 134 | self.assertEqual(mock_urlopen.call_count, 1) |
| 135 | finally: |
| 136 | release.set() |
| 137 | |
| 138 | @patch("lib.http.urllib.request.urlopen") |
| 139 | def test_error_body_within_deadline_keeps_status_and_body(self, mock_urlopen): |
| 140 | mock_urlopen.side_effect = urllib.error.HTTPError( |
| 141 | "https://example.com", 401, "Unauthorized", {}, io.BytesIO(b"bad token") |
| 142 | ) |
| 143 | with self.assertRaises(http.HTTPError) as caught: |
| 144 | http.request( |
| 145 | "GET", "https://example.com", retries=1, |
| 146 | deadline_monotonic=time.monotonic() + 1, |
| 147 | ) |
| 148 | self.assertEqual(caught.exception.status_code, 401) |
| 149 | self.assertEqual(caught.exception.body, "bad token") |
| 150 | self.assertEqual(caught.exception.outcome_state, http.health.AUTH_FAILED) |
| 151 | |
| 152 | @patch("lib.http.urllib.request.urlopen") |
| 153 | def test_worker_socket_timeout_is_not_wall_deadline_expiration( |
| 154 | self, |
| 155 | mock_urlopen, |
| 156 | ): |
| 157 | mock_urlopen.side_effect = TimeoutError("early socket timeout") |
| 158 | |
| 159 | with self.assertRaises(http.HTTPError) as caught: |
| 160 | http.request( |
| 161 | "GET", |
| 162 | "http://example.com", |
| 163 | retries=1, |
| 164 | deadline_monotonic=time.monotonic() + 600, |
| 165 | ) |
| 166 | |
| 167 | self.assertNotIsInstance(caught.exception, http.DeadlineExceeded) |
| 168 | self.assertEqual(http.health.TIMEOUT, caught.exception.outcome_state) |
| 169 | |
| 170 | |
| 171 | def _mock_response(body: str = '{"ok": true}', status: int = 200): |
| 172 | resp = MagicMock() |
| 173 | resp.__enter__ = MagicMock(return_value=resp) |
| 174 | resp.__exit__ = MagicMock(return_value=False) |
| 175 | resp.read.return_value = body.encode("utf-8") |
| 176 | resp.status = status |
| 177 | return resp |
| 178 | |
| 179 | |
| 180 | class TestParamsEncoding(unittest.TestCase): |
| 181 | """request() should urlencode the params dict into the URL.""" |
| 182 | |
| 183 | def _sent_url(self, mock_urlopen) -> str: |
| 184 | request_arg = mock_urlopen.call_args[0][0] |
| 185 | return request_arg.full_url |
| 186 | |
| 187 | @patch("lib.http.urllib.request.urlopen") |
| 188 | def test_params_appended_to_url(self, mock_urlopen): |
| 189 | mock_urlopen.return_value = _mock_response() |
| 190 | http.get("https://api.example.com/search", params={"q": "test", "limit": 10}) |
| 191 | sent_url = self._sent_url(mock_urlopen) |
| 192 | self.assertIn("q=test", sent_url) |
| 193 | self.assertIn("limit=10", sent_url) |
| 194 | |
| 195 | @patch("lib.http.urllib.request.urlopen") |
| 196 | def test_params_appended_with_existing_query_string(self, mock_urlopen): |
| 197 | mock_urlopen.return_value = _mock_response() |
| 198 | http.get("https://api.example.com/search?api_key=secret", params={"q": "test"}) |
| 199 | sent_url = self._sent_url(mock_urlopen) |
| 200 | self.assertTrue(sent_url.startswith("https://api.example.com/search?api_key=secret&")) |
| 201 | self.assertIn("q=test", sent_url) |
| 202 | |
| 203 | @patch("lib.http.urllib.request.urlopen") |
| 204 | def test_none_values_dropped(self, mock_urlopen): |
| 205 | mock_urlopen.return_value = _mock_response() |
| 206 | http.get("https://api.example.com/search", params={"q": "test", "filter": None}) |
| 207 | sent_url = self._sent_url(mock_urlopen) |
| 208 | self.assertIn("q=test", sent_url) |
| 209 | self.assertNotIn("filter", sent_url) |
| 210 | |
| 211 | @patch("lib.http.urllib.request.urlopen") |
| 212 | def test_empty_params_leaves_url_unchanged(self, mock_urlopen): |
| 213 | mock_urlopen.return_value = _mock_response() |
| 214 | http.get("https://api.example.com/search", params={}) |
| 215 | sent_url = self._sent_url(mock_urlopen) |
| 216 | self.assertEqual(sent_url, "https://api.example.com/search") |
| 217 | |
| 218 | @patch("lib.http.urllib.request.urlopen") |
| 219 | def test_no_params_kwarg_leaves_url_unchanged(self, mock_urlopen): |
| 220 | mock_urlopen.return_value = _mock_response() |
| 221 | http.get("https://api.example.com/search") |
| 222 | sent_url = self._sent_url(mock_urlopen) |
| 223 | self.assertEqual(sent_url, "https://api.example.com/search") |
| 224 | |
| 225 | @patch("lib.http.urllib.request.urlopen") |
| 226 | def test_int_and_bool_params_stringified(self, mock_urlopen): |
| 227 | mock_urlopen.return_value = _mock_response() |
| 228 | http.get("https://api.example.com/search", params={"count": 25, "raw": True}) |
| 229 | sent_url = self._sent_url(mock_urlopen) |
| 230 | self.assertIn("count=25", sent_url) |
| 231 | self.assertIn("raw=True", sent_url) |
| 232 | |
| 233 | |
| 234 | class TestDNSResolutionRetry(unittest.TestCase): |
| 235 | """DNS resolution failures (gaierror) must retry with exponential backoff. |
| 236 | |
| 237 | Caller-passed `retries` values smaller than MIN_DNS_RETRIES are expanded |
| 238 | on the first gaierror so a transient resolution failure doesn't wipe a |
| 239 | request just because the caller passed retries=2. |
| 240 | """ |
| 241 | |
| 242 | @patch("lib.http.urllib.request.urlopen") |
| 243 | @patch("lib.http.time.sleep") |
| 244 | def test_gaierror_retries_up_to_min_dns_retries_even_when_caller_passes_fewer( |
| 245 | self, mock_sleep, mock_urlopen |
| 246 | ): |
| 247 | """Caller passed retries=2; gaierror should still get MIN_DNS_RETRIES attempts.""" |
| 248 | import socket |
| 249 | err = urllib.error.URLError(socket.gaierror(-2, "Name or service not known")) |
| 250 | mock_urlopen.side_effect = err |
| 251 | |
| 252 | with self.assertRaises(http.HTTPError): |
| 253 | http.request("GET", "http://nonexistent.example", retries=2) |
| 254 | |
| 255 | # Caller passed retries=2, but the budget expanded to MIN_DNS_RETRIES=3. |
| 256 | self.assertEqual(mock_urlopen.call_count, http.MIN_DNS_RETRIES) |
| 257 | |
| 258 | @patch("lib.http.urllib.request.urlopen") |
| 259 | @patch("lib.http.time.sleep") |
| 260 | def test_dns_failure_widens_single_attempt_budget(self, mock_sleep, mock_urlopen): |
| 261 | """DNS failures widen the budget to MIN_DNS_RETRIES, even from retries=1.""" |
| 262 | import socket |
| 263 | mock_urlopen.side_effect = urllib.error.URLError(socket.gaierror(-2, "DNS failure")) |
| 264 | |
| 265 | with self.assertRaises(http.HTTPError): |
| 266 | http.request("GET", "https://example.com", retries=1) |
| 267 | |
| 268 | self.assertEqual(mock_urlopen.call_count, http.MIN_DNS_RETRIES) |
| 269 | |
| 270 | @patch("lib.http.urllib.request.urlopen") |
| 271 | @patch("lib.http.time.sleep") |
| 272 | def test_gaierror_succeeds_after_transient_failure(self, mock_sleep, mock_urlopen): |
| 273 | """gaierror on attempt 1, then success — should NOT raise.""" |
| 274 | import socket |
| 275 | success_response = MagicMock() |
| 276 | success_response.read.return_value = b'{"ok": true}' |
| 277 | success_response.status = 200 |
| 278 | success_response.__enter__ = lambda self: self |
| 279 | success_response.__exit__ = lambda *args: None |
| 280 | |
| 281 | err = urllib.error.URLError(socket.gaierror(-2, "Name or service not known")) |
| 282 | mock_urlopen.side_effect = [err, success_response] |
| 283 | |
| 284 | result = http.request("GET", "http://flaky.example", retries=2) |
| 285 | |
| 286 | self.assertEqual(result, {"ok": True}) |
| 287 | self.assertEqual(mock_urlopen.call_count, 2) |
| 288 | |
| 289 | @patch("lib.http.urllib.request.urlopen") |
| 290 | @patch("lib.http.time.sleep") |
| 291 | def test_gaierror_uses_exponential_backoff(self, mock_sleep, mock_urlopen): |
| 292 | """Backoff delays for gaierror should be 1s, 2s, 4s — not the linear default.""" |
| 293 | import socket |
| 294 | err = urllib.error.URLError(socket.gaierror(-2, "Name or service not known")) |
| 295 | mock_urlopen.side_effect = err |
| 296 | |
| 297 | with self.assertRaises(http.HTTPError): |
| 298 | http.request("GET", "http://nonexistent.example", retries=3) |
| 299 | |
| 300 | # Expected sleep calls: 1s (after attempt 1), 2s (after attempt 2). |
| 301 | # No sleep after the final attempt (the loop exits to raise). |
| 302 | sleep_delays = [call.args[0] for call in mock_sleep.call_args_list] |
| 303 | self.assertEqual(sleep_delays, [1, 2]) |
| 304 | |
| 305 | @patch("lib.http.urllib.request.urlopen") |
| 306 | @patch("lib.http.time.sleep") |
| 307 | def test_non_dns_urlerror_uses_linear_backoff_not_dns_branch( |
| 308 | self, mock_sleep, mock_urlopen |
| 309 | ): |
| 310 | """A URLError that's NOT a gaierror must NOT expand the retry budget.""" |
| 311 | # ConnectionRefusedError-style URLError reason (not gaierror) |
| 312 | err = urllib.error.URLError(ConnectionRefusedError(111, "Connection refused")) |
| 313 | mock_urlopen.side_effect = err |
| 314 | |
| 315 | with self.assertRaises(http.HTTPError): |
| 316 | http.request("GET", "http://refused.example", retries=2) |
| 317 | |
| 318 | # Caller passed retries=2, and non-DNS URLError doesn't expand it. |
| 319 | self.assertEqual(mock_urlopen.call_count, 2) |
| 320 | |
| 321 | @patch("lib.http.urllib.request.urlopen") |
| 322 | @patch("lib.http.time.sleep") |
| 323 | def test_dns_widening_does_not_leak_into_subsequent_non_dns_urlerror( |
| 324 | self, mock_sleep, mock_urlopen |
| 325 | ): |
| 326 | """Mixed sequence: DNS-then-non-DNS must respect caller's original retries. |
| 327 | |
| 328 | Without the fix, the first gaierror widens effective_retries from 2 to |
| 329 | MIN_DNS_RETRIES=3, and a subsequent ConnectionRefused on attempt 1 |
| 330 | slips into a third overall attempt — exceeding what the caller asked |
| 331 | for. Each non-DNS error path must gate on the original `retries`. |
| 332 | """ |
| 333 | import socket |
| 334 | dns_err = urllib.error.URLError(socket.gaierror(-2, "Name or service not known")) |
| 335 | conn_err = urllib.error.URLError(ConnectionRefusedError(111, "Connection refused")) |
| 336 | mock_urlopen.side_effect = [dns_err, conn_err, conn_err] # 3rd would only fire if budget leaked |
| 337 | |
| 338 | with self.assertRaises(http.HTTPError): |
| 339 | http.request("GET", "http://flaky.example", retries=2) |
| 340 | |
| 341 | # Caller asked for at most 2 attempts. DNS widening must not give us a 3rd. |
| 342 | self.assertEqual(mock_urlopen.call_count, 2) |
| 343 | |
| 344 | @patch("lib.http.urllib.request.urlopen") |
| 345 | @patch("lib.http.time.sleep") |
| 346 | def test_dns_widening_does_not_leak_into_subsequent_oserror( |
| 347 | self, mock_sleep, mock_urlopen |
| 348 | ): |
| 349 | """Mixed sequence: DNS-then-OSError must respect caller's original retries.""" |
| 350 | import socket |
| 351 | dns_err = urllib.error.URLError(socket.gaierror(-2, "Name or service not known")) |
| 352 | mock_urlopen.side_effect = [dns_err, TimeoutError("timed out"), TimeoutError("timed out")] |
| 353 | |
| 354 | with self.assertRaises(http.HTTPError): |
| 355 | http.request("GET", "http://flaky.example", retries=2) |
| 356 | |
| 357 | self.assertEqual(mock_urlopen.call_count, 2) |
| 358 | |
| 359 | |
| 360 | class TestDebugLogRedaction(unittest.TestCase): |
| 361 | """Debug log lines must not echo credentials the request carried.""" |
| 362 | |
| 363 | SECRET = "sk-live-abcdef1234567890" |
| 364 | |
| 365 | def _logged(self, mock_log) -> str: |
| 366 | return "\n".join(call.args[0] for call in mock_log.call_args_list) |
| 367 | |
| 368 | @patch("lib.http.log") |
| 369 | @patch("lib.http.urllib.request.urlopen") |
| 370 | def test_error_body_echoing_bearer_token_is_redacted(self, mock_urlopen, mock_log): |
| 371 | mock_urlopen.side_effect = urllib.error.HTTPError( |
| 372 | "https://example.com", 401, "Unauthorized", {}, |
| 373 | io.BytesIO(f"invalid key Bearer {self.SECRET}".encode("utf-8")), |
| 374 | ) |
| 375 | |
| 376 | with self.assertRaises(http.HTTPError): |
| 377 | http.request( |
| 378 | "POST", "https://example.com", retries=1, |
| 379 | headers={"Authorization": f"Bearer {self.SECRET}"}, |
| 380 | json_data={"q": "x"}, |
| 381 | ) |
| 382 | |
| 383 | logged = self._logged(mock_log) |
| 384 | self.assertIn("Error body:", logged) |
| 385 | self.assertIn("<redacted>", logged) |
| 386 | self.assertNotIn(self.SECRET, logged) |
| 387 | |
| 388 | @patch("lib.http.log") |
| 389 | @patch("lib.http.urllib.request.urlopen") |
| 390 | @patch("lib.http.time.sleep") |
| 391 | def test_url_error_echoing_api_key_is_redacted(self, _mock_sleep, mock_urlopen, mock_log): |
| 392 | mock_urlopen.side_effect = urllib.error.URLError(f"proxy refused key {self.SECRET}") |
| 393 | |
| 394 | with self.assertRaises(http.HTTPError): |
| 395 | http.request( |
| 396 | "GET", "https://example.com", retries=1, |
| 397 | headers={"X-Api-Key": self.SECRET}, |
| 398 | ) |
| 399 | |
| 400 | logged = self._logged(mock_log) |
| 401 | self.assertIn("URL Error:", logged) |
| 402 | self.assertNotIn(self.SECRET, logged) |
| 403 |