fix(search): complete the query error surface across every endpoint

Four pieces of the same surface:

whoosh-compat's emit() documents a two-part host contract: both a parse
diagnostic and the QueryEmitError/UnsupportedQueryError pair are
user-input errors. Only the latter half was caught; QueryEmitError now
maps to SearchQueryError too. Messages pass through a cleanup that
strips the library's DIVERGENCES.md references and replaces the
fast=True host-configuration advice with user language, so no
library-internal vocabulary reaches a searching user.

The bulk selection paths (bulk edit, the legacy bulk endpoint, bulk
download) reached the backend with no SearchQueryError handler, so a
bad date or number in a selection filter raised straight to a DRF 500.
They now share the search list endpoint's exact mapping (a new
search_query_error_messages helper flattens MultipleSearchQueryErrors
in one place), returning the same 400 body for the same bad query.

QueryParserError means a whoosh-compat parser bug, not user-fixable
input, per its own contract; the list endpoint's blanket handler was
converting it to a generic 400. It now re-raises and surfaces as a 500
that monitoring can see.

All behavior is pinned test-first: bulk edit and bulk download API
tests assert 400s naming the bad value (previously unhandled
exceptions), a unit test pins the QueryEmitError mapping, three
parametrized checks assert no internal vocabulary leaks for the
unsupported query shapes, and a mocked parser-bug test asserts the 500.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WMsn6DgzbvSqh1pwy66VVF
This commit is contained in:
Trenton Holmes
2026-08-19 13:36:53 -07:00
committed by stumpylog
co-authored by Claude Fable 5
parent 98edd75220
commit efb6e4b0f1
7 changed files with 180 additions and 19 deletions
+53
View File
@@ -11,6 +11,7 @@ import time_machine
from documents.search._query import InvalidDateQuery
from documents.search._query import InvalidNumberQuery
from documents.search._query import MultipleSearchQueryErrors
from documents.search._query import SearchQueryError
from documents.search._query import build_permission_filter
from documents.search._query import parse_simple_text_highlight_query
from documents.search._query import parse_user_query
@@ -333,3 +334,55 @@ class TestSearchQueryErrors:
assert err.errors == tuple(sub_errors)
assert "created" in str(err)
assert "asn" in str(err)
class TestEmitErrorContract:
"""whoosh-compat's emit() documents a two-part host contract: BOTH a
non-empty diagnostics list AND the QueryEmitError/UnsupportedQueryError
pair raised by emit() itself are user-input errors. Every one must
surface as SearchQueryError (HTTP 400), with library-internal
vocabulary (DIVERGENCES.md references, fast=True host advice) kept out
of the user-facing message."""
@pytest.fixture
def query_index(self) -> tantivy.Index:
schema = build_schema()
idx = tantivy.Index(schema, path=None)
register_tokenizers(idx, "")
return idx
def test_query_emit_error_maps_to_search_query_error(
self,
query_index: tantivy.Index,
monkeypatch: pytest.MonkeyPatch,
) -> None:
from whoosh_compat.errors import QueryEmitError
import documents.search._query as query_mod
def raise_emit_error(*args: object, **kwargs: object) -> None:
raise QueryEmitError("synthetic emit failure")
monkeypatch.setattr(query_mod, "tantivy_emit", raise_emit_error)
with pytest.raises(SearchQueryError):
parse_user_query(query_index, "invoice", UTC)
@pytest.mark.parametrize(
("query", "leaked_fragment"),
[
pytest.param("title:[a TO b]", "DIVERGENCES", id="text-range-doc-ref"),
pytest.param("notes.note:wild*", "DIVERGENCES", id="json-wildcard-doc-ref"),
pytest.param("notes.user:*", "fast=True", id="exists-host-advice"),
],
)
def test_unsupported_messages_carry_no_internal_vocabulary(
self,
query_index: tantivy.Index,
query: str,
leaked_fragment: str,
) -> None:
with pytest.raises(SearchQueryError) as exc_info:
parse_user_query(query_index, query, UTC)
assert leaked_fragment not in str(exc_info.value)
# The message must still say something useful, not be blanked.
assert str(exc_info.value).strip()
@@ -339,3 +339,21 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase):
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.content, b"Insufficient permissions")
def test_bad_search_query_returns_400(self) -> None:
response = self.client.post(
self.ENDPOINT,
json.dumps(
{
"all": True,
"filters": {"query": "added:notadate"},
"content": "originals",
},
),
content_type="application/json",
)
# A user-fixable query error must surface as a 400 naming the bad
# value, exactly like the search list endpoint, never a 500.
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(b"notadate", response.content)
+19
View File
@@ -1976,3 +1976,22 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(LogEntry.objects.filter(object_pk=self.doc1.id).count(), 2)
def test_api_bulk_edit_with_bad_search_query_returns_400(self) -> None:
response = self.client.post(
"/api/documents/bulk_edit/",
json.dumps(
{
"all": True,
"filters": {"query": "added:notadate"},
"method": "set_storage_path",
"parameters": {"storage_path": self.sp1.id},
},
),
content_type="application/json",
)
# A user-fixable query error must surface as a 400 naming the bad
# value, exactly like the search list endpoint, never a 500.
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(b"notadate", response.content)
+23
View File
@@ -855,6 +855,29 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase):
results = response.data["results"]
self.assertEqual({r["id"] for r in results}, {1, 2})
@mock.patch("documents.search._backend.parse_user_query")
def test_search_parser_bug_surfaces_as_500_not_400(self, m) -> None:
"""
GIVEN:
- The query parser itself fails (a whoosh-compat bug, per
QueryParserError's own contract: not user-fixable input)
WHEN:
- Any search request runs
THEN:
- The error surfaces as a 500 monitoring can see, never a 400
blaming the user for a library defect
"""
from whoosh_compat.errors import QueryParserError
m.side_effect = QueryParserError("synthetic parser bug")
self.client.raise_request_exception = False
response = self.client.get("/api/documents/?query=anything")
self.assertEqual(
response.status_code,
status.HTTP_500_INTERNAL_SERVER_ERROR,
)
@mock.patch("documents.search._backend.TantivyBackend.autocomplete")
def test_search_autocomplete_limits(self, m) -> None:
"""