Skip to content

Commit 6a55068

Browse files
committed
fix(ctl): classify 4xx marketplace errors as exit 1 and keep show --json clean
1 parent 15dca4a commit 6a55068

2 files changed

Lines changed: 111 additions & 1 deletion

File tree

infrahub_sdk/ctl/marketplace.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ async def _run_listing(
168168
try:
169169
items, total_count = await _fetch_listing(client, resolved_url, item_type, search=search, limit=limit)
170170
except httpx.HTTPError as exc:
171+
error_class = _classify_http_error(exc)
172+
if error_class is _ErrorClass.NOT_FOUND:
173+
_fail(error_class, f"Marketplace listing not found on {_host_from(resolved_url)}: {exc}")
171174
detail = str(exc) or type(exc).__name__
172175
_fail(_ErrorClass.NETWORK, f"Marketplace request failed: {detail}")
173176
if json_output:
@@ -182,6 +185,19 @@ def _is_transport_failure(r: object) -> bool:
182185
return isinstance(r, httpx.Response) and r.status_code >= 500
183186

184187

188+
def _classify_http_error(exc: httpx.HTTPError) -> _ErrorClass:
189+
"""Map an httpx error to an error class for consistent exit-code assignment.
190+
191+
A response with a 4xx status code is a client/not-found error (exit 1).
192+
A 5xx response or a transport-level failure with no response is a network
193+
error (exit 2).
194+
"""
195+
response = getattr(exc, "response", None)
196+
if response is not None and response.status_code < 500:
197+
return _ErrorClass.NOT_FOUND
198+
return _ErrorClass.NETWORK
199+
200+
185201
def _mkdir_or_fail(path: Path) -> None:
186202
try:
187203
path.mkdir(parents=True, exist_ok=True)
@@ -415,7 +431,7 @@ async def _fetch_detail(
415431
)
416432
if isinstance(schema_resp, httpx.Response) and schema_resp.status_code == 200:
417433
if isinstance(collection_resp, httpx.Response) and collection_resp.status_code == 200:
418-
console.print(
434+
err_console.print(
419435
f"[yellow]Note: '{namespace}/{name}' exists as both a schema and a collection. "
420436
"Resolving as schema. Pass --collection to force the collection path."
421437
)
@@ -509,6 +525,9 @@ async def show(
509525
client, resolved_url, parsed.namespace, parsed.name, force_collection=collection
510526
)
511527
except httpx.HTTPError as exc:
528+
error_class = _classify_http_error(exc)
529+
if error_class is _ErrorClass.NOT_FOUND:
530+
_fail(error_class, f"Marketplace listing not found on {_host_from(resolved_url)}: {exc}")
512531
message = str(exc) or type(exc).__name__
513532
_fail(_ErrorClass.NETWORK, f"Marketplace request failed: {message}")
514533
if json_output:

tests/unit/ctl/test_marketplace_app.py

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,3 +921,94 @@ async def test_collection_false_autodetects_schema(httpx_mock: HTTPXMock, tmp_pa
921921
)
922922

923923
assert (tmp_path / "network-base.yml").read_text() == SCHEMA_YAML
924+
925+
926+
# ---------------------------------------------------------------------------
927+
# Fix #1 — 4xx errors must exit 1 (not 2)
928+
# ---------------------------------------------------------------------------
929+
930+
931+
def test_list_4xx_error_exits_1(httpx_mock: HTTPXMock) -> None:
932+
"""A 4xx response on the listing endpoint must exit 1, not 2."""
933+
httpx_mock.add_response(
934+
method="GET",
935+
url="https://marketplace.infrahub.app/api/v1/schemas",
936+
status_code=400,
937+
json={"detail": "Bad Request"},
938+
)
939+
result = runner.invoke(app, ["list"])
940+
941+
assert result.exit_code == 1
942+
943+
944+
def test_show_force_collection_4xx_exits_1(httpx_mock: HTTPXMock) -> None:
945+
"""A 4xx response on the --collection (force) path in show must exit 1, not 2."""
946+
httpx_mock.add_response(
947+
method="GET",
948+
url="https://marketplace.infrahub.app/api/v1/collections/infrahub/security-mgmt",
949+
status_code=400,
950+
json={"detail": "Bad Request"},
951+
)
952+
result = runner.invoke(app, ["show", "infrahub/security-mgmt", "--collection"])
953+
954+
assert result.exit_code == 1
955+
956+
957+
# ---------------------------------------------------------------------------
958+
# Fix #2 — collision note must not appear on stdout when --json is used
959+
# ---------------------------------------------------------------------------
960+
961+
962+
def test_show_collision_json_stdout_is_clean(httpx_mock: HTTPXMock) -> None:
963+
"""When show hits a 200/200 collision and --json is passed, stdout must stay JSON-only.
964+
965+
The collision note must be routed to err_console (stderr) not console (stdout).
966+
967+
Stream-separation assertion limitation: Typer 0.25 / Click 8.3 do not
968+
support ``mix_stderr=False`` on CliRunner, so we cannot isolate stdout in
969+
this test runner. The structural fix (console.print → err_console.print in
970+
``_fetch_detail``) is the authoritative correction; this test guards that the
971+
command exits 0 on a collision+json path and that the JSON payload is
972+
present somewhere in the combined output.
973+
"""
974+
httpx_mock.add_response(
975+
method="GET",
976+
url="https://marketplace.infrahub.app/api/v1/schemas/infrahub/vlan",
977+
json=_schema_detail(),
978+
)
979+
httpx_mock.add_response(
980+
method="GET",
981+
url="https://marketplace.infrahub.app/api/v1/collections/infrahub/vlan",
982+
json=_collection_detail(),
983+
)
984+
985+
result = runner.invoke(app, ["show", "infrahub/vlan", "--json"])
986+
987+
assert result.exit_code == 0
988+
# The combined output must contain the JSON payload. We parse the whole
989+
# block starting from the first '{' at column 0 (the root object printed by
990+
# Rich's print_json always starts at column 0).
991+
output = result.output
992+
root_brace = next((i for i, ch in enumerate(output) if ch == "{" and (i == 0 or output[i - 1] == "\n")), None)
993+
assert root_brace is not None, "No root-level JSON object found in output"
994+
parsed = _json.loads(output[root_brace:])
995+
assert parsed["name"] == "vlan"
996+
997+
998+
# ---------------------------------------------------------------------------
999+
# Regression guard — 5xx must still exit 2
1000+
# ---------------------------------------------------------------------------
1001+
1002+
1003+
def test_list_network_error_exits_2_regression(httpx_mock: HTTPXMock) -> None:
1004+
"""503 responses must still exit 2 (unchanged behaviour)."""
1005+
httpx_mock.add_response(
1006+
method="GET",
1007+
url="https://marketplace.infrahub.app/api/v1/schemas",
1008+
status_code=503,
1009+
json={"detail": "unavailable"},
1010+
)
1011+
result = runner.invoke(app, ["list"])
1012+
1013+
assert result.exit_code == 2
1014+
assert "Marketplace request failed" in result.output

0 commit comments

Comments
 (0)