Skip to content

Commit 0f2f6da

Browse files
g-despotclaude
andcommitted
fix(grpc-web): guard grpc-web mode (shim+async required), tighten timeout/fallback
Addresses the second Copilot review on #2056: - base.py: grpc_path_prefix now fails fast in _grpc_channel when used on a sync client or when the weaviate-python-grpc-web shim is not active, instead of silently building a native grpcio channel that ignores the prefix. - helpers.py: connect_to_custom (sync) rejects a non-empty grpc_path_prefix and points to use_async_with_custom; the docstring is clarified that grpc-web is async-only. - _channel.py: _encode_timeout rounds the grpc-timeout up (math.ceil) so we never advertise a shorter deadline than requested. - proto/v1/__init__.py: the grpcio metadata fallback is restricted to Emscripten, so a broken/partial grpcio install on a normal platform surfaces as PackageNotFoundError instead of being masked by a fallback stub version. Tests added/updated (sync/no-shim rejection, off-emscripten raise, grpc-timeout round-up). End-to-end against a vanguard transcoder on a shared host:port still passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 833bb59 commit 0f2f6da

7 files changed

Lines changed: 115 additions & 14 deletions

File tree

packages/grpc-web/src/weaviate_grpc_web/_channel.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import asyncio
1616
import base64
17+
import math
1718
import urllib.parse
1819
from typing import Any, Callable, Dict, Optional
1920

@@ -37,10 +38,12 @@ def get_sender() -> Sender:
3738

3839
def _encode_timeout(seconds: float) -> str:
3940
"""Encode a timeout as a grpc-timeout header value (``<positive int><unit>``)."""
40-
millis = max(1, int(seconds * 1000))
41+
# Round up so we never advertise a shorter deadline than requested (which would risk
42+
# premature server-side cancellation); grpc-timeout takes a positive integer + unit.
43+
millis = max(1, math.ceil(seconds * 1000))
4144
if millis < 100_000_000:
4245
return f"{millis}m"
43-
return f"{max(1, int(seconds))}S"
46+
return f"{max(1, math.ceil(seconds))}S"
4447

4548

4649
def _fold_metadata(headers: Dict[str, str], metadata: Any) -> None:

packages/grpc-web/tests/test_transport.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,15 @@ def test_malformed_grpc_status_maps_to_internal():
180180
assert excinfo.value.code() is StatusCode.INTERNAL
181181

182182

183+
def test_grpc_timeout_header_rounds_up():
184+
sender = FakeSender(body=_ok_response(b"x"))
185+
channel = _channel(sender)
186+
mc = channel.unary_unary("/svc/M", lambda x: x, lambda b: b)
187+
# 123.4ms must round UP to 124ms (never advertise a shorter deadline than requested).
188+
asyncio.run(mc(b"q", timeout=0.1234))
189+
assert sender.calls[0][1]["grpc-timeout"] == "124m"
190+
191+
183192
def test_close_is_awaitable_noop():
184193
channel = _channel(FakeSender())
185194
assert asyncio.run(channel.close()) is None

proto_test/test_proto.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,32 @@ def raises(pkg: str) -> str:
6060
raise PackageNotFoundError(pkg)
6161

6262
monkeypatch.setattr(mod, "metadata_version", raises)
63+
monkeypatch.setattr("sys.platform", "emscripten")
6364

6465
assert str(mod.get_version("grpcio")) == "1.72.1"
6566
with pytest.raises(PackageNotFoundError):
6667
mod.get_version("protobuf")
6768

6869

70+
@pytest.mark.skipif(
71+
_INCOMPATIBLE_GRPC_PB,
72+
reason="weaviate.proto.v1 cannot be imported with an incompatible grpcio/protobuf "
73+
"pair (CI version-gate matrix); the gate is covered by test_proto_import and the "
74+
"fallback is exercised in every compatible cell",
75+
)
76+
def test_grpcio_missing_metadata_raises_off_emscripten(monkeypatch):
77+
"""Off Emscripten, missing grpcio metadata surfaces instead of being masked."""
78+
mod = importlib.import_module("weaviate.proto.v1")
79+
80+
def raises(pkg: str) -> str:
81+
raise PackageNotFoundError(pkg)
82+
83+
monkeypatch.setattr(mod, "metadata_version", raises)
84+
monkeypatch.setattr("sys.platform", "linux")
85+
with pytest.raises(PackageNotFoundError):
86+
mod.get_version("grpcio")
87+
88+
6989
@pytest.mark.skipif(
7090
_INCOMPATIBLE_GRPC_PB,
7191
reason="weaviate.proto.v1 cannot be imported with an incompatible grpcio/protobuf "

test/test_connection_params.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import weaviate.connect.base as base_mod
55
from weaviate.connect.base import ConnectionParams
6+
from weaviate.exceptions import WeaviateInvalidInputError
67

78

89
def test_same_host_port_raises_without_prefix() -> None:
@@ -88,6 +89,8 @@ def fake_insecure_channel(target, options=None, **kwargs):
8889
return "CHANNEL"
8990

9091
monkeypatch.setattr(base_mod.grpc.aio, "insecure_channel", fake_insecure_channel)
92+
# grpc-web mode requires the shim to be active; simulate it being installed.
93+
monkeypatch.setattr(base_mod.grpc, "__weaviate_grpc_web_shim__", True, raising=False)
9194

9295
params = ConnectionParams.from_params(
9396
http_host="localhost",
@@ -105,6 +108,48 @@ def fake_insecure_channel(target, options=None, **kwargs):
105108
assert ("grpc-web.path_prefix", "/grpc-web") in captured["options"]
106109

107110

111+
def _grpc_web_params() -> ConnectionParams:
112+
return ConnectionParams.from_params(
113+
http_host="localhost",
114+
http_port=8090,
115+
http_secure=False,
116+
grpc_host="localhost",
117+
grpc_port=8090,
118+
grpc_secure=False,
119+
grpc_path_prefix="/grpc-web",
120+
)
121+
122+
123+
def test_grpc_channel_rejects_prefix_without_shim(monkeypatch) -> None:
124+
# No grpc-web shim active -> must fail fast instead of silently building a native
125+
# grpcio channel that ignores the prefix.
126+
monkeypatch.delattr(base_mod.grpc, "__weaviate_grpc_web_shim__", raising=False)
127+
with pytest.raises(WeaviateInvalidInputError, match="weaviate-python-grpc-web"):
128+
_grpc_web_params()._grpc_channel(proxies={}, grpc_msg_size=None, is_async=True)
129+
130+
131+
def test_grpc_channel_rejects_prefix_for_sync_client() -> None:
132+
# grpc-web is async-only; a sync channel with a prefix must be rejected.
133+
with pytest.raises(WeaviateInvalidInputError, match="async"):
134+
_grpc_web_params()._grpc_channel(proxies={}, grpc_msg_size=None, is_async=False)
135+
136+
137+
def test_connect_to_custom_rejects_grpc_web_prefix() -> None:
138+
# The synchronous helper must reject grpc-web up front (before connecting).
139+
import weaviate
140+
141+
with pytest.raises(WeaviateInvalidInputError, match="async-only"):
142+
weaviate.connect_to_custom(
143+
http_host="localhost",
144+
http_port=8080,
145+
http_secure=False,
146+
grpc_host="localhost",
147+
grpc_port=8080,
148+
grpc_secure=False,
149+
grpc_path_prefix="/grpc-web",
150+
)
151+
152+
108153
def test_grpc_channel_omits_option_without_prefix(monkeypatch) -> None:
109154
captured: dict = {}
110155

weaviate/connect/base.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from pydantic import BaseModel, field_validator, model_validator
1010

1111
from weaviate.config import GrpcConfig, Proxies
12+
from weaviate.exceptions import WeaviateInvalidInputError
1213
from weaviate.types import NUMBER
1314
from weaviate.util import is_weaviate_domain
1415

@@ -160,10 +161,24 @@ def _grpc_channel(
160161
if grpc_config is not None and grpc_config.channel_options is not None:
161162
options.extend(grpc_config.channel_options)
162163

163-
# In grpc-web mode, forward the base-path prefix to the transport via channel
164-
# options (consumed by the weaviate-python-grpc-web shim). Not added for native
165-
# gRPC, so the native channel options stay byte-for-byte unchanged.
164+
# grpc-web mode (prefix set): only valid for an async client, and only when the
165+
# weaviate-python-grpc-web shim has replaced the grpc module (it consumes the
166+
# grpc-web.path_prefix option). Fail fast otherwise — a native grpcio channel
167+
# would silently ignore the option and route over native gRPC, a confusing
168+
# misconfiguration. Nothing is added for native gRPC, so its channel options stay
169+
# byte-for-byte unchanged.
166170
if (prefix := self._grpc_web_path_prefix) != "":
171+
if not is_async:
172+
raise WeaviateInvalidInputError(
173+
"grpc_path_prefix (grpc-web) is only supported for async clients; "
174+
"use use_async_with_custom(...) / WeaviateAsyncClient"
175+
)
176+
if not getattr(grpc, "__weaviate_grpc_web_shim__", False):
177+
raise WeaviateInvalidInputError(
178+
"grpc_path_prefix enables grpc-web, which requires the "
179+
"'weaviate-python-grpc-web' package (it installs a grpc shim before "
180+
"'import weaviate'); it is not active in this environment"
181+
)
167182
options.append(("grpc-web.path_prefix", prefix))
168183

169184
if is_async:

weaviate/connect/helpers.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from weaviate.config import AdditionalConfig
1818
from weaviate.connect.base import ConnectionParams, ProtocolParams
1919
from weaviate.embedded import WEAVIATE_VERSION, EmbeddedOptions
20+
from weaviate.exceptions import WeaviateInvalidInputError
2021
from weaviate.util import docstring_deprecated
2122
from weaviate.validator import _validate_input, _ValidateArgument
2223
from weaviate.warnings import _Warnings
@@ -313,11 +314,11 @@ def connect_to_custom(
313314
a bearer token, in which case use `weaviate.classes.init.Auth.bearer_token()`, a client secret, in which case use `weaviate.classes.init.Auth.client_credentials()`
314315
or a username and password, in which case use `weaviate.classes.init.Auth.client_password()`.
315316
skip_init_checks: Whether to skip the initialization checks when connecting to Weaviate.
316-
grpc_path_prefix: Optional base-path prefix for a grpc-web endpoint served on the
317-
same host:port as REST (e.g. "/grpc-web"). When set, gRPC requests are sent
318-
over grpc-web to ``<scheme>://<grpc_host>:<grpc_port><prefix>/...`` and sharing
319-
the REST host:port is allowed. Requires the ``weaviate-python-grpc-web``
320-
package. Defaults to None (native gRPC).
317+
grpc_path_prefix: grpc-web base-path prefix. grpc-web is async-only, so it is NOT
318+
supported by the synchronous ``connect_to_custom`` — passing a non-empty value
319+
raises ``WeaviateInvalidInputError``. Use
320+
``use_async_with_custom(..., grpc_path_prefix=...)`` instead. Defaults to None
321+
(native gRPC).
321322
322323
Returns:
323324
The client connected to the instance with the required parameters set appropriately.
@@ -350,6 +351,11 @@ def connect_to_custom(
350351
True
351352
>>> # The connection is automatically closed when the context is exited.
352353
"""
354+
if grpc_path_prefix:
355+
raise WeaviateInvalidInputError(
356+
"grpc_path_prefix enables grpc-web, which is async-only; use "
357+
"use_async_with_custom(...) instead of connect_to_custom(...)"
358+
)
353359
return __connect(
354360
WeaviateClient(
355361
ConnectionParams.from_params(

weaviate/proto/v1/__init__.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import sys
12
import warnings
23

34

@@ -19,16 +20,18 @@
1920
# This happens under Pyodide/Emscripten, where grpcio has no wheel and is excluded
2021
# via the `sys_platform != "emscripten"` marker in setup.cfg; the grpc module itself
2122
# is provided there by a pure-Python shim (see the weaviate-python-grpc-web package).
22-
# On every normal install grpcio's metadata is present and the real version is used,
23-
# so this branch is not taken. Restricted to grpcio so that a genuinely missing
24-
# protobuf (which is required and pure-Python under Pyodide) is never masked.
23+
# On every normal install grpcio's metadata is present and the real version is used, so
24+
# this branch is not taken. Restricted to grpcio AND to Emscripten, so that a broken or
25+
# partial grpcio install on a normal platform (metadata missing) still surfaces as
26+
# PackageNotFoundError instead of silently selecting a fallback stub, and so a genuinely
27+
# missing protobuf (required and pure-Python under Pyodide) is never masked.
2528
_GRPCIO_FALLBACK_VERSION = "1.72.1"
2629

2730
def get_version(pkg: str) -> version.Version:
2831
try:
2932
return version.parse(metadata_version(pkg))
3033
except PackageNotFoundError:
31-
if pkg == "grpcio":
34+
if pkg == "grpcio" and sys.platform == "emscripten":
3235
return version.parse(_GRPCIO_FALLBACK_VERSION)
3336
raise
3437

0 commit comments

Comments
 (0)