Route SCHEMA_FIELD_MISSING into an error, not an HTTP 400

This commit is contained in:
stumpylog
2026-09-13 15:28:41 -07:00
committed by GitHub
parent baed2b4a9d
commit b9f2eea469
3 changed files with 48 additions and 26 deletions
+12 -7
View File
@@ -43,7 +43,8 @@ def _user_facing_emit_message(d: Diagnostic) -> str:
Built from the Diagnostic's structured fields (kind, field), never from Built from the Diagnostic's structured fields (kind, field), never from
d.message: whoosh-compat documents that as developer/log output with no d.message: whoosh-compat documents that as developer/log output with no
stability guarantee, and PATTERN_TOO_COMPLEX embeds the raw backend stability guarantee, and PATTERN_TOO_COMPLEX embeds the raw backend
error text in it. error text in it. SCHEMA_FIELD_MISSING never reaches here: _map_emit_error
re-raises it before calling this function, the same as INTERNAL.
""" """
field = str(d.field) if d.field is not None else None field = str(d.field) if d.field is not None else None
if d.kind is DiagnosticKind.EXISTS_REQUIRES_FAST: if d.kind is DiagnosticKind.EXISTS_REQUIRES_FAST:
@@ -52,8 +53,6 @@ def _user_facing_emit_message(d: Diagnostic) -> str:
return f"Range searches are not supported for field {field!r}." return f"Range searches are not supported for field {field!r}."
if d.kind is DiagnosticKind.PATTERN_TOO_COMPLEX: if d.kind is DiagnosticKind.PATTERN_TOO_COMPLEX:
return f"The wildcard pattern for field {field!r} is too complex." return f"The wildcard pattern for field {field!r} is too complex."
if d.kind is DiagnosticKind.SCHEMA_FIELD_MISSING:
return f"Field {field!r} is not available in the search index."
logger.warning( logger.warning(
"Unmapped emit diagnostic %s: %s", "Unmapped emit diagnostic %s: %s",
d.kind, d.kind,
@@ -69,10 +68,15 @@ def _map_emit_error(e: QueryError) -> SearchQueryError:
diagnostic, and map to a 400. INTERNAL means a defect in whoosh-compat diagnostic, and map to a 400. INTERNAL means a defect in whoosh-compat
or in our own AST handling, never the user's query, so the QueryError is or in our own AST handling, never the user's query, so the QueryError is
re-raised rather than converted, reaching the generic 500 handler instead re-raised rather than converted, reaching the generic 500 handler instead
of blaming the query. MISCONFIGURED is deliberately both: the registry and of blaming the query. MISCONFIGURED other than EXISTS_REQUIRES_FAST is
the index schema disagree, which only an operator can fix, so it is logged treated the same way as INTERNAL: the registry and the index schema
as an error, but a request is still waiting and the query cannot run disagree, which only an operator can fix, and the exact same query would
either way, so it also returns a 400. succeed on its own once the index is rebuilt. That makes it a transient
server-side condition, not a permanently bad request, so it is logged as
an error and re-raised rather than converted to a 400: telling the
client their query is invalid would be wrong, it would work once the
index catches up, and a 400 also hides the condition from monitoring
that only watches 5xx rates.
EXISTS_REQUIRES_FAST is the one MISCONFIGURED kind that is not a EXISTS_REQUIRES_FAST is the one MISCONFIGURED kind that is not a
disagreement. whoosh-compat derives it from the registry's own FieldSpec disagreement. whoosh-compat derives it from the registry's own FieldSpec
@@ -96,6 +100,7 @@ def _map_emit_error(e: QueryError) -> SearchQueryError:
d.kind.name, d.kind.name,
d.message, d.message,
) )
raise e
return SearchQueryError(_user_facing_emit_message(d)) return SearchQueryError(_user_facing_emit_message(d))
@@ -82,7 +82,7 @@ class TestEmitErrorRouting:
_map_emit_error(error) _map_emit_error(error)
assert excinfo.value is error assert excinfo.value is error
def test_misconfigured_cause_is_logged_and_becomes_a_400( def test_misconfigured_cause_is_logged_and_reraised(
self, self,
caplog: pytest.LogCaptureFixture, caplog: pytest.LogCaptureFixture,
) -> None: ) -> None:
@@ -92,15 +92,21 @@ class TestEmitErrorRouting:
WHEN: WHEN:
- _map_emit_error processes it - _map_emit_error processes it
THEN: THEN:
- It becomes a SearchQueryError, and exactly one ERROR log - Exactly one ERROR log record is emitted naming the field and
record is emitted naming the field and the diagnostic kind the diagnostic kind, and the original QueryError propagates
unchanged: a registry/schema disagreement is transient (the
exact same query succeeds once the index is rebuilt), so it
surfaces as a 500 an operator can see rather than a 400
telling the client their query is permanently invalid
""" """
kind = DiagnosticKind.SCHEMA_FIELD_MISSING kind = DiagnosticKind.SCHEMA_FIELD_MISSING
with caplog.at_level(logging.ERROR, logger="paperless.search"): error = QueryError(_diagnostic(kind, field=FieldRef("asn")))
error = _map_emit_error( with (
QueryError(_diagnostic(kind, field=FieldRef("asn"))), caplog.at_level(logging.ERROR, logger="paperless.search"),
) pytest.raises(QueryError) as excinfo,
assert isinstance(error, SearchQueryError) ):
_map_emit_error(error)
assert excinfo.value is error
errors = [r for r in caplog.records if r.levelno == logging.ERROR] errors = [r for r in caplog.records if r.levelno == logging.ERROR]
assert len(errors) == 1 assert len(errors) == 1
assert "asn" in errors[0].getMessage() assert "asn" in errors[0].getMessage()
@@ -144,7 +150,6 @@ class TestEmitErrorRouting:
DiagnosticKind.TEXT_RANGE, DiagnosticKind.TEXT_RANGE,
DiagnosticKind.PATTERN_TOO_COMPLEX, DiagnosticKind.PATTERN_TOO_COMPLEX,
DiagnosticKind.EXISTS_REQUIRES_FAST, DiagnosticKind.EXISTS_REQUIRES_FAST,
DiagnosticKind.SCHEMA_FIELD_MISSING,
], ],
) )
def test_user_facing_message_never_echoes_library_prose( def test_user_facing_message_never_echoes_library_prose(
@@ -154,7 +159,10 @@ class TestEmitErrorRouting:
""" """
GIVEN: GIVEN:
- A QueryError carrying whoosh-compat's own developer-facing - A QueryError carrying whoosh-compat's own developer-facing
message text message text (SCHEMA_FIELD_MISSING excluded: it is now
re-raised rather than converted, so it never produces a
user-facing message at all, see
test_misconfigured_cause_is_logged_and_reraised)
WHEN: WHEN:
- _map_emit_error processes it - _map_emit_error processes it
THEN: THEN:
@@ -170,7 +178,6 @@ class TestEmitErrorRouting:
DiagnosticKind.TEXT_RANGE, DiagnosticKind.TEXT_RANGE,
DiagnosticKind.PATTERN_TOO_COMPLEX, DiagnosticKind.PATTERN_TOO_COMPLEX,
DiagnosticKind.EXISTS_REQUIRES_FAST, DiagnosticKind.EXISTS_REQUIRES_FAST,
DiagnosticKind.SCHEMA_FIELD_MISSING,
], ],
) )
def test_user_facing_message_names_the_field( def test_user_facing_message_names_the_field(
@@ -9,7 +9,10 @@ action can clear the condition. Any authenticated user could otherwise emit
ERROR lines in a loop by repeating ``notes:*``. ERROR lines in a loop by repeating ``notes:*``.
SCHEMA_FIELD_MISSING, the other MISCONFIGURED kind, does compare the registry SCHEMA_FIELD_MISSING, the other MISCONFIGURED kind, does compare the registry
against the live schema, so it stays an ERROR. against the live schema, so it stays an ERROR log. But it is not a 400
either: the exact same query would succeed once the index is rebuilt, so it
is a transient server-side condition, not a permanently bad request, and is
re-raised the same way an INTERNAL cause is.
""" """
from __future__ import annotations from __future__ import annotations
@@ -81,7 +84,7 @@ class TestJsonExistsIsUserError:
class TestGenuineMisconfigurationStillLogs: class TestGenuineMisconfigurationStillLogs:
def test_schema_field_missing_is_an_error_log( def test_schema_field_missing_is_an_error_log_and_reraised(
self, self,
caplog: pytest.LogCaptureFixture, caplog: pytest.LogCaptureFixture,
) -> None: ) -> None:
@@ -92,9 +95,13 @@ class TestGenuineMisconfigurationStillLogs:
WHEN: WHEN:
- _map_emit_error processes it - _map_emit_error processes it
THEN: THEN:
- It becomes a SearchQueryError and logs exactly one ERROR - It logs exactly one ERROR record naming the diagnostic kind
record naming the diagnostic kind, since this is a real (a real mismatch an operator can fix, so it keeps the
mismatch an operator can fix and keeps the alert alert), and the original QueryError propagates unchanged
rather than becoming a SearchQueryError: the exact same
query would succeed once the index is rebuilt, so this is a
transient server-side condition, not a permanently bad
request, and surfaces as a 500 rather than a 400
""" """
kind = DiagnosticKind.SCHEMA_FIELD_MISSING kind = DiagnosticKind.SCHEMA_FIELD_MISSING
error = QueryError( error = QueryError(
@@ -106,9 +113,12 @@ class TestGenuineMisconfigurationStillLogs:
field_kind=FieldKind.U64, field_kind=FieldKind.U64,
), ),
) )
with caplog.at_level(logging.ERROR, logger="paperless.search"): with (
mapped = _map_emit_error(error) caplog.at_level(logging.ERROR, logger="paperless.search"),
assert isinstance(mapped, SearchQueryError) pytest.raises(QueryError) as excinfo,
):
_map_emit_error(error)
assert excinfo.value is error
records = [r for r in caplog.records if r.levelno == logging.ERROR] records = [r for r in caplog.records if r.levelno == logging.ERROR]
assert len(records) == 1 assert len(records) == 1
assert kind.name in records[0].getMessage() assert kind.name in records[0].getMessage()