diff --git a/src/ucode/databricks.py b/src/ucode/databricks.py index a0accb9be..dc03a4124 100644 --- a/src/ucode/databricks.py +++ b/src/ucode/databricks.py @@ -26,6 +26,7 @@ from decimal import Decimal, InvalidOperation from email.message import Message from enum import Enum +from http.client import HTTPException from pathlib import Path from typing import Any, Literal, NamedTuple, NoReturn, cast, overload from urllib import error as urllib_error @@ -378,11 +379,9 @@ def _http_get_bytes( if attempt == max_retries: return None, reason retry_after = None - except OSError as exc: - # A socket read timeout raises a bare TimeoutError (an OSError), not a - # URLError, so it must be caught explicitly or it escapes the whole - # discovery flow. Surface it as a reason like every other failure. - _debug(f"GET {url}", f"OSError: {exc}") + except (OSError, HTTPException) as exc: + # Socket timeouts and truncated HTTP responses can escape urllib unwrapped. + _debug(f"GET {url}", f"{type(exc).__name__}: {exc}") reason = f"network error: {exc}" if attempt == max_retries: return None, reason @@ -486,10 +485,9 @@ def _http_send_json( except urllib_error.URLError as exc: _debug(f"{method} {url}", f"URLError: {exc.reason}") return None, f"network error: {exc.reason}" - except OSError as exc: - # See `_http_get_json`: a bare socket timeout is an OSError, not a - # URLError, and would otherwise escape the caller's error handling. - _debug(f"{method} {url}", f"OSError: {exc}") + except (OSError, HTTPException) as exc: + # A failed response read does not establish whether the write committed. + _debug(f"{method} {url}", f"{type(exc).__name__}: {exc}") return None, f"network error: {exc}" diff --git a/tests/README.md b/tests/README.md index a78179707..56631d215 100644 --- a/tests/README.md +++ b/tests/README.md @@ -548,7 +548,11 @@ Bedrock integration journey still supplies live Windows/Claude coverage. These are actual argv/configuration assertions, not native classifier acceptance coverage. `test_databricks.py` checks bounded Anthropic catalog requests with `limit=1000`, including -scoped routing headers and model display metadata. It also verifies that Windows CLI install +scoped routing headers and model display metadata. HTTP component cases cover truncated or +malformed responses, successful GET recovery, and exhausted retry budgets. Write-response +failures return a reason without replaying a potentially committed request. These cases +inject transport failures; they do not establish live gateway recovery. +It also verifies that Windows CLI install and upgrade use WinGet and report an actionable error when WinGet is unavailable. Windows PATH refresh regressions prime a cached missing CLI, then verify that existing WinGet aliases and persisted user PATH entries are discovered without reinstalling, including repeated bootstrap. diff --git a/tests/test_databricks.py b/tests/test_databricks.py index bcf26da70..e47d609bc 100644 --- a/tests/test_databricks.py +++ b/tests/test_databricks.py @@ -2,6 +2,7 @@ from __future__ import annotations +import http.client import json import os import subprocess @@ -10,6 +11,7 @@ import time from decimal import Decimal from pathlib import Path +from unittest.mock import Mock from urllib.parse import parse_qs import pytest @@ -4075,6 +4077,59 @@ def fake_urlopen(request, timeout=None): assert len(calls) == 3 assert sleeps == [1.25, 2.5] + @pytest.mark.parametrize("max_retries", [0, 2]) + @pytest.mark.parametrize( + "failure", + [http.client.IncompleteRead(b'{"partial":', 100), http.client.BadStatusLine("bad status")], + ids=["truncated-body", "bad-status"], + ) + def test_http_protocol_failure_returns_reason_after_retry_budget( + self, monkeypatch, max_retries, failure + ): + response = _FakeResponse(b"") + response.read = Mock(side_effect=failure) + opener = Mock(return_value=response) + sleeps = Mock() + monkeypatch.setattr(db_mod.urllib_request, "urlopen", opener) + monkeypatch.setattr(db_mod.time, "sleep", sleeps) + + payload, reason = db_mod._http_get_json("https://x/y", "tok", max_retries=max_retries) + + assert payload is None + assert reason == f"network error: {failure}" + assert opener.call_count == max_retries + 1 + assert sleeps.call_count == max_retries + + def test_retries_truncated_body_and_returns_only_the_complete_response(self, monkeypatch): + truncated = _FakeResponse(b"") + truncated.read = Mock(side_effect=http.client.IncompleteRead(b'{"data":', 100)) + opener = Mock(side_effect=[truncated, _FakeResponse({"data": ["complete"]})]) + sleeps = [] + monkeypatch.setattr(db_mod.urllib_request, "urlopen", opener) + monkeypatch.setattr(db_mod.random, "uniform", lambda low, high: high) + monkeypatch.setattr(db_mod.time, "sleep", sleeps.append) + + payload, reason = db_mod._http_get_json("https://x/y", "tok", max_retries=2) + + assert payload == {"data": ["complete"]} + assert reason is None + assert opener.call_count == 2 + assert sleeps == [1.25] + + @pytest.mark.parametrize("method", ["POST", "PATCH", "DELETE"]) + def test_truncated_write_response_returns_reason_without_replaying(self, monkeypatch, method): + failure = http.client.IncompleteRead(b'{"partial":', 100) + response = _FakeResponse(b"") + response.read = Mock(side_effect=failure) + opener = Mock(return_value=response) + monkeypatch.setattr(db_mod.urllib_request, "urlopen", opener) + + payload, reason = db_mod._http_send_json(method, "https://x/y", "tok", {"key": "value"}) + + assert payload is None + assert reason == f"network error: {failure}" + assert opener.call_count == 1 + def test_does_not_retry_other_http_error(self, monkeypatch): calls = []