Skip to content

Commit 195c382

Browse files
minitrigaclaude
andcommitted
refactor(ctl): dedupe marketplace probe/error handling, guard listing payload
Address review findings on the marketplace browsing commands: - Guard `_fetch_listing` against a valid-JSON-but-non-object body so `list`/`search` report a clean network error (exit 2) instead of dumping a traceback and corrupting `--json` output. Matches the isinstance guard the collection download path already applies. - Extract the parallel schema/collection probe (parallel gather, "schema wins a 200/200 collision", transport-vs-not-found classification) into `_probe_item_type`, shared by `_detect_item_type` (get) and `_fetch_detail` (show), so detection semantics live in one place. - Extract the duplicated httpx error classification into `_fail_http_error`, shared by `list`/`search` and `show`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3df0fd8 commit 195c382

2 files changed

Lines changed: 68 additions & 41 deletions

File tree

infrahub_sdk/ctl/marketplace.py

Lines changed: 55 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ async def _fetch_listing(
106106
resp = await client.get(url, params=params)
107107
resp.raise_for_status()
108108
payload = _json_or_fail(resp, url)
109+
if not isinstance(payload, dict):
110+
_fail(_ErrorClass.NETWORK, f"Response from {_host_from(url)} is not a valid marketplace listing.")
109111
items.extend(payload.get("items", []))
110112
total_count = payload.get("total_count", len(items))
111113
page_info = payload.get("page_info") or {}
@@ -172,11 +174,7 @@ async def _run_listing(
172174
try:
173175
items, total_count = await _fetch_listing(client, resolved_url, item_type, search=search, limit=limit)
174176
except httpx.HTTPError as exc:
175-
error_class = _classify_http_error(exc)
176-
if error_class is _ErrorClass.NOT_FOUND:
177-
_fail(error_class, f"Marketplace request to {_host_from(resolved_url)} failed: {exc}")
178-
detail = str(exc) or type(exc).__name__
179-
_fail(_ErrorClass.NETWORK, f"Marketplace request failed: {detail}")
177+
_fail_http_error(exc, f"Marketplace request to {_host_from(resolved_url)} failed: {exc}")
180178
if json_output:
181179
_print_json(items)
182180
return
@@ -202,6 +200,14 @@ def _classify_http_error(exc: httpx.HTTPError) -> _ErrorClass:
202200
return _ErrorClass.NETWORK
203201

204202

203+
def _fail_http_error(exc: httpx.HTTPError, not_found_message: str) -> NoReturn:
204+
"""Fail with the right exit code for an httpx error: 4xx → exit 1, 5xx/transport → exit 2."""
205+
if _classify_http_error(exc) is _ErrorClass.NOT_FOUND:
206+
_fail(_ErrorClass.NOT_FOUND, not_found_message)
207+
detail = str(exc) or type(exc).__name__
208+
_fail(_ErrorClass.NETWORK, f"Marketplace request failed: {detail}")
209+
210+
205211
def _json_or_fail(resp: httpx.Response, source_url: str) -> Any:
206212
"""Parse a JSON response body, failing with a network-class error on invalid JSON.
207213
@@ -236,22 +242,24 @@ def _make_http_client(sdk_cfg: _SdkConfig) -> httpx.AsyncClient:
236242
return httpx.AsyncClient(follow_redirects=True, verify=sdk_cfg.tls_context, **proxy_kwargs)
237243

238244

239-
async def _detect_item_type(
245+
async def _probe_item_type(
240246
client: httpx.AsyncClient,
241247
base_url: str,
242248
namespace: str,
243249
name: str,
244250
*,
245-
stdout: bool,
251+
schema_url: str,
252+
collection_url: str,
253+
collision_console: Console,
246254
) -> tuple[MarketplaceItemType, httpx.Response]:
247-
"""Probe schema and collection endpoints in parallel. Schema wins on 200-200.
255+
"""Probe the given schema and collection URLs in parallel. Schema wins on 200-200.
248256
249257
Returns the resolved type and the winning 200 response so the caller can reuse
250-
it instead of re-fetching the same URL.
258+
it instead of re-fetching the same URL. On a 200-200 collision the note is
259+
printed to ``collision_console`` (stderr, so it never pollutes stdout/--json).
260+
Fails with a network error on transport failure and not-found when neither
261+
endpoint returns 200.
251262
"""
252-
schema_url = _schema_url(base_url, namespace, name)
253-
collection_url = _collection_url(base_url, namespace, name)
254-
255263
schema_resp, collection_resp = await asyncio.gather(
256264
client.get(schema_url),
257265
client.get(collection_url),
@@ -260,7 +268,7 @@ async def _detect_item_type(
260268

261269
if isinstance(schema_resp, httpx.Response) and schema_resp.status_code == 200:
262270
if isinstance(collection_resp, httpx.Response) and collection_resp.status_code == 200:
263-
_status_console(stdout).print(
271+
collision_console.print(
264272
f"[yellow]Note: '{namespace}/{name}' exists as both a schema and a collection. "
265273
"Resolving as schema. Pass --collection to force the collection path."
266274
)
@@ -280,6 +288,30 @@ async def _detect_item_type(
280288
)
281289

282290

291+
async def _detect_item_type(
292+
client: httpx.AsyncClient,
293+
base_url: str,
294+
namespace: str,
295+
name: str,
296+
*,
297+
stdout: bool,
298+
) -> tuple[MarketplaceItemType, httpx.Response]:
299+
"""Auto-detect whether ``namespace/name`` is a schema or collection via the download endpoints.
300+
301+
Returns the resolved type and the winning 200 response so the caller can reuse
302+
it instead of re-fetching the same URL.
303+
"""
304+
return await _probe_item_type(
305+
client,
306+
base_url,
307+
namespace,
308+
name,
309+
schema_url=_schema_url(base_url, namespace, name),
310+
collection_url=_collection_url(base_url, namespace, name),
311+
collision_console=_status_console(stdout),
312+
)
313+
314+
283315
async def _download_schema(
284316
client: httpx.AsyncClient,
285317
base_url: str,
@@ -441,30 +473,16 @@ async def _fetch_detail(
441473
resp.raise_for_status()
442474
return "collection", _json_or_fail(resp, base_url)
443475

444-
schema_resp, collection_resp = await asyncio.gather(
445-
client.get(_detail_url(base_url, "schema", namespace, name)),
446-
client.get(_detail_url(base_url, "collection", namespace, name)),
447-
return_exceptions=True,
448-
)
449-
if isinstance(schema_resp, httpx.Response) and schema_resp.status_code == 200:
450-
if isinstance(collection_resp, httpx.Response) and collection_resp.status_code == 200:
451-
err_console.print(
452-
f"[yellow]Note: '{namespace}/{name}' exists as both a schema and a collection. "
453-
"Resolving as schema. Pass --collection to force the collection path."
454-
)
455-
return "schema", _json_or_fail(schema_resp, base_url)
456-
if isinstance(collection_resp, httpx.Response) and collection_resp.status_code == 200:
457-
return "collection", _json_or_fail(collection_resp, base_url)
458-
459-
if _is_transport_failure(schema_resp) or _is_transport_failure(collection_resp):
460-
_fail(
461-
_ErrorClass.NETWORK,
462-
f"Could not reach marketplace at {base_url}. Check your connection or --marketplace-url.",
463-
)
464-
_fail(
465-
_ErrorClass.NOT_FOUND,
466-
f"No schema or collection named '{namespace}/{name}' found on {_host_from(base_url)}.",
476+
item_type, resp = await _probe_item_type(
477+
client,
478+
base_url,
479+
namespace,
480+
name,
481+
schema_url=_detail_url(base_url, "schema", namespace, name),
482+
collection_url=_detail_url(base_url, "collection", namespace, name),
483+
collision_console=err_console,
467484
)
485+
return item_type, _json_or_fail(resp, base_url)
468486

469487

470488
def _render_detail(detail: dict[str, Any], item_type: MarketplaceItemType) -> None:
@@ -542,11 +560,7 @@ async def show(
542560
client, resolved_url, parsed.namespace, parsed.name, force_collection=collection
543561
)
544562
except httpx.HTTPError as exc:
545-
error_class = _classify_http_error(exc)
546-
if error_class is _ErrorClass.NOT_FOUND:
547-
_fail(error_class, f"Marketplace request for '{parsed.namespace}/{parsed.name}' failed: {exc}")
548-
message = str(exc) or type(exc).__name__
549-
_fail(_ErrorClass.NETWORK, f"Marketplace request failed: {message}")
563+
_fail_http_error(exc, f"Marketplace request for '{parsed.namespace}/{parsed.name}' failed: {exc}")
550564
if json_output:
551565
_print_json(detail)
552566
return

tests/unit/ctl/test_marketplace_app.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,19 @@ def test_list_invalid_json_body_is_network_error(httpx_mock: HTTPXMock) -> None:
10541054
assert "not valid JSON" in result.output
10551055

10561056

1057+
def test_list_non_dict_payload_is_network_error(httpx_mock: HTTPXMock) -> None:
1058+
"""A 200 response whose body is valid JSON but not an object exits 2 cleanly, not a traceback."""
1059+
httpx_mock.add_response(
1060+
method="GET",
1061+
url="https://marketplace.infrahub.app/api/v1/schemas",
1062+
json=[{"namespace": "infrahub", "name": "dcim"}],
1063+
)
1064+
result = runner.invoke(app, ["list"])
1065+
1066+
assert result.exit_code == 2
1067+
assert "not a valid marketplace listing" in result.output
1068+
1069+
10571070
def test_show_invalid_json_body_is_network_error(httpx_mock: HTTPXMock) -> None:
10581071
"""A schema detail endpoint returning 200 with a malformed body exits 2, not a traceback."""
10591072
httpx_mock.add_response(

0 commit comments

Comments
 (0)