This is an automated email from the ASF dual-hosted git repository. cgivre pushed a commit to branch feat/drill-mcp-server in repository https://gitbox.apache.org/repos/asf/drill-mcp.git
commit 6ea9a5311c24a5aa9f08a11161b905da6cc02b61 Author: cgivre <[email protected]> AuthorDate: Wed Aug 12 15:20:43 2026 -0400 revert: stop suppressing Drill's error text on the probe path The premise for the earlier sanitisation (Drill errors can embed sampled cell content) was fabricated and unverified; the Drill maintainer confirmed Drill's errors do not do this. The probe's failure now propagates Drill's error text unchanged, same as _describe_columns and fetch_plugin_type, so a missing table, a permissions failure, and a genuine data error are distinguishable again instead of collapsing into one opaque message. --- drill_mcp/client_rest.py | 25 ++++++++----------------- tests/test_client_rest.py | 31 ++++++++++++++++--------------- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/drill_mcp/client_rest.py b/drill_mcp/client_rest.py index bffb88a..2165ba0 100644 --- a/drill_mcp/client_rest.py +++ b/drill_mcp/client_rest.py @@ -370,14 +370,13 @@ def _probe_columns(query: Query, schema: str, table: str, plugin_type: str) -> l caller (`describe_table`) gets column names and types, never sampled values. - That same privacy property is why a probe FAILURE is handled specially - below, not just left to propagate: Drill's own error text for a query - that fails while reading a row (a type-coercion or malformed-record - error, for instance) can embed the offending cell's content -- - `_error_text` passes `errorMessage` through verbatim, and `server.py` - surfaces `DrillError`'s text to the caller unchanged. `DESCRIBE` could - never trigger this path; only the probe can, so only the probe needs to - guard against it. + A probe FAILURE is left to propagate unchanged, exactly like + `_describe_columns` and `fetch_plugin_type`: Drill's error text (a + missing table, a permissions failure, a genuine data error) is what a + caller needs to tell those apart and correct the request. Confirmed with + the Drill maintainer that Drill's own errors do not embed cell content, + so there is nothing here for a probe-specific failure path to guard + against. """ if plugin_type == "mongo": # MongoDB collection names CAN contain dots (e.g. "logs.2024"); this @@ -393,15 +392,7 @@ def _probe_columns(query: Query, schema: str, table: str, plugin_type: str) -> l target = _probe_target(schema, table) sql = f"SELECT * FROM {target} LIMIT 1" - try: - result = query(sql, 1) - except DrillError as exc: - # Deliberately does NOT include `exc`'s text in the new message -- - # only chains it as the cause -- because that text may be Drill's - # own error message, which can embed sampled cell content. - raise DrillError( - f"could not determine columns for `{schema}`.`{table}`: the probe query failed" - ) from exc + result = query(sql, 1) if not result.columns: # A dynamic-schema plugin discovers columns only by reading data; diff --git a/tests/test_client_rest.py b/tests/test_client_rest.py index 59d894e..983be61 100644 --- a/tests/test_client_rest.py +++ b/tests/test_client_rest.py @@ -619,29 +619,30 @@ class TestFilePluginMetadata: assert "078-05-1120-SENTINEL" not in repr(result) @respx.mock - def test_columns_probe_failure_does_not_leak_drills_error_text(self): - # A probe reads a row -- if Drill fails WHILE reading it (a - # type-coercion or malformed-record error), Drill's own error text - # can embed the offending cell's content. DESCRIBE could never - # trigger this; only the probe can, so the probe's failure path must - # not surface Drill's raw error message. + def test_columns_probe_failure_surfaces_drills_error_text_unchanged(self): + # Drill's own error text is what a caller needs to tell a missing + # table, a permissions failure, and a genuine data error apart, and + # to correct the request -- so the probe path propagates it exactly + # like `_describe_columns` and `fetch_plugin_type` already do. (An + # earlier version of this code suppressed it here on the theory that + # Drill's error text could embed sampled cell content; the Drill + # maintainer confirmed that premise was wrong, so this test protects + # the opposite property.) respx.post(f"{BASE}/query.json").mock( side_effect=[ self._schemata("file"), httpx.Response( - 500, json={"errorMessage": "conversion failed on value SENTINEL-CELL-9182"} + 500, + json={ + "errorMessage": "VALIDATION ERROR: Object 'sales.csv' not found within 'dfs.tmp'" + }, ), ] ) - with pytest.raises(DrillError) as exc_info: + with pytest.raises( + DrillError, match=r"Object 'sales\.csv' not found within 'dfs\.tmp'" + ): make_client().columns("dfs.tmp", "sales.csv") - message = str(exc_info.value) - assert "SENTINEL-CELL-9182" not in message - assert "dfs.tmp" in message - assert "sales.csv" in message - # The original Drill error is preserved as the exception chain, just - # not folded into the new message text. - assert "SENTINEL-CELL-9182" in str(exc_info.value.__cause__) @respx.mock def test_columns_probe_raises_when_the_table_is_empty(self):
