Skip to content

Commit 7fc0ff3

Browse files
rustyconoverclaude
andcommitted
fix(client): surface a malformed catalog_contents item as CatalogClientError
contents() decoded the response after leaving the _catalog_connect() block, so a malformed item (an invalid IPC payload, an unknown enum value) escaped as a bare ArrowInvalid / KeyError instead of CatalogClientError. load_catalog then neither fell back to the per-schema RPCs nor raised the client's error type. Decode inside the block, like every other catalog call. Found by vgi-polars against vgi-fixture-bad-enum-worker. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 6b3182b commit 7fc0ff3

2 files changed

Lines changed: 63 additions & 7 deletions

File tree

‎tests/catalog/test_client_load_catalog.py‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
from collections.abc import Iterator
1414
from contextlib import contextmanager
15+
from types import SimpleNamespace
1516
from typing import Any
1617

1718
import pytest
@@ -31,7 +32,7 @@
3132
SchemaInfo,
3233
TransactionOpaqueData,
3334
)
34-
from vgi.client import CatalogClientMixin, CatalogContents, CatalogSnapshot
35+
from vgi.client import CatalogClientMixin, CatalogContents, CatalogSnapshot, catalog_mixin
3536
from vgi.client.catalog_mixin import CatalogClientError
3637

3738
_KINDS = (
@@ -333,3 +334,55 @@ def test_absent_supports_catalog_contents_decodes_false() -> None:
333334
assert CatalogAttachResult.deserialize_from_batch(batch).supports_catalog_contents
334335
old = batch.drop_columns(["supports_catalog_contents"])
335336
assert not CatalogAttachResult.deserialize_from_batch(old).supports_catalog_contents
337+
338+
339+
class _UndecodableContentsProxy:
340+
"""A worker proxy whose ``catalog_contents`` item bytes are not a valid ``SchemaInfo``."""
341+
342+
def catalog_contents(self, **_: Any) -> Any:
343+
entry = SimpleNamespace(
344+
path=["main"],
345+
schema=b"not an arrow ipc stream",
346+
tables=[],
347+
views=[],
348+
scalar_functions=[],
349+
aggregate_functions=[],
350+
table_functions=[],
351+
scalar_macros=[],
352+
table_macros=[],
353+
indexes=[],
354+
)
355+
return SimpleNamespace(catalog_version=1, etag=None, not_modified=False, schemas=[entry])
356+
357+
def catalog_schemas(self, **_: Any) -> Any:
358+
return SimpleNamespace(to_infos=list)
359+
360+
361+
@pytest.fixture
362+
def undecodable_contents_client(monkeypatch: pytest.MonkeyPatch) -> CatalogClientMixin:
363+
"""A client whose real ``_catalog_connect`` hands out `_UndecodableContentsProxy`."""
364+
365+
@contextmanager
366+
def connect(*_: Any, **__: Any) -> Iterator[Any]:
367+
yield _UndecodableContentsProxy()
368+
369+
monkeypatch.setattr(catalog_mixin._catalog_pool, "connect", connect)
370+
371+
class _Client(CatalogClientMixin):
372+
def _worker_argv(self) -> list[str]:
373+
return ["unused"]
374+
375+
return _Client()
376+
377+
378+
def test_undecodable_contents_raises_catalog_client_error(undecodable_contents_client: CatalogClientMixin) -> None:
379+
"""A decode failure surfaces as ``CatalogClientError``, like every other catalog call."""
380+
with pytest.raises(CatalogClientError):
381+
undecodable_contents_client.contents(attach_opaque_data=_ATTACH)
382+
383+
384+
def test_undecodable_contents_falls_back_to_per_schema(undecodable_contents_client: CatalogClientMixin) -> None:
385+
"""``load_catalog`` treats an undecodable answer as a failure and uses the per-schema RPCs."""
386+
snapshot = undecodable_contents_client.load_catalog(attach=_attach_result(True))
387+
assert snapshot.source == "per_schema"
388+
assert snapshot.fallback_reason

‎vgi/client/catalog_mixin.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -541,14 +541,17 @@ def contents(self, *, attach_opaque_data: AttachOpaqueData, if_none_match: str |
541541
one [`SchemaContentsInfo`][] per schema, parents first.
542542
543543
"""
544+
# Decode inside the connection block so a malformed item surfaces as
545+
# CatalogClientError (which load_catalog falls back on), not a bare
546+
# decode error.
544547
with self._catalog_connect() as proxy:
545548
response = proxy.catalog_contents(attach_opaque_data=attach_opaque_data, if_none_match=if_none_match)
546-
return CatalogContents(
547-
catalog_version=response.catalog_version,
548-
etag=response.etag,
549-
not_modified=response.not_modified,
550-
schemas=[_decode_schema_contents(entry) for entry in response.schemas],
551-
)
549+
return CatalogContents(
550+
catalog_version=response.catalog_version,
551+
etag=response.etag,
552+
not_modified=response.not_modified,
553+
schemas=[_decode_schema_contents(entry) for entry in response.schemas],
554+
)
552555

553556
def load_catalog(
554557
self,

0 commit comments

Comments
 (0)