diff --git a/docsgpt/deploy/commands.py b/docsgpt/deploy/commands.py index a62bee78..758fc715 100644 --- a/docsgpt/deploy/commands.py +++ b/docsgpt/deploy/commands.py @@ -815,12 +815,13 @@ def _endpoint(url: str) -> str: """A URL without its credentials: this ends up on a terminal, in CI logs and in issues.""" try: parts = urlsplit(url) + # .port is a property that parses on access, so it raises separately from the split itself. + host, port, scheme, path = parts.hostname or "", parts.port, parts.scheme, parts.path except ValueError: return "the configured URL" - host = parts.hostname or "" - if parts.port: - host = f"{host}:{parts.port}" - return f"{parts.scheme}://{host}{parts.path}" if host else "the configured URL" + if port: + host = f"{host}:{port}" + return f"{scheme}://{host}{path}" if host else "the configured URL" def _scrub(text: str, url: Optional[str]) -> str: diff --git a/tests/deploy/test_doctor.py b/tests/deploy/test_doctor.py index 185f8c4f..ac046a85 100644 --- a/tests/deploy/test_doctor.py +++ b/tests/deploy/test_doctor.py @@ -328,8 +328,26 @@ class TestRedisCheck: assert "default" not in check.detail assert "redis.example.com:6380/0" in check.detail, "the endpoint still has to be identifiable" - def test_a_malformed_url_is_not_echoed_either(self): - assert commands._endpoint("redis://[::1") == "the configured URL" + @pytest.mark.parametrize("url", ["redis://[::1", "redis://localhost:not-a-port/0"]) + def test_a_malformed_url_is_not_echoed_either(self, url): + """urlsplit raises on the first; on the second it succeeds and .port raises on access.""" + assert commands._endpoint(url) == "the configured URL" + + def test_a_failure_on_a_url_with_an_unparsable_port_is_still_a_check(self, monkeypatch): + """The check catches the client error and then formats it, which is where this raised.""" + import redis + + def from_url(cls, url, **kwargs): + class Client: + def ping(self): + raise ConnectionError(f"no route to {url}") + + return Client() + + monkeypatch.setattr(redis.Redis, "from_url", classmethod(from_url)) + check = commands._check_redis({"broker": "redis://localhost:not-a-port/0"}) + assert check.level == "fail", "a bad port is a finding, not a traceback" + assert "the configured URL" in check.detail def test_without_any_redis_configured(self): assert commands._check_redis({}).level == "fail"