mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-09-13 13:17:59 +00:00
Route SCHEMA_FIELD_MISSING into an error, not an HTTP 400
This commit is contained in:
@@ -43,7 +43,8 @@ def _user_facing_emit_message(d: Diagnostic) -> str:
|
||||
Built from the Diagnostic's structured fields (kind, field), never from
|
||||
d.message: whoosh-compat documents that as developer/log output with no
|
||||
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
|
||||
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}."
|
||||
if d.kind is DiagnosticKind.PATTERN_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(
|
||||
"Unmapped emit diagnostic %s: %s",
|
||||
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
|
||||
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
|
||||
of blaming the query. MISCONFIGURED is deliberately both: the registry and
|
||||
the index schema disagree, which only an operator can fix, so it is logged
|
||||
as an error, but a request is still waiting and the query cannot run
|
||||
either way, so it also returns a 400.
|
||||
of blaming the query. MISCONFIGURED other than EXISTS_REQUIRES_FAST is
|
||||
treated the same way as INTERNAL: the registry and the index schema
|
||||
disagree, which only an operator can fix, and the exact same query would
|
||||
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
|
||||
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.message,
|
||||
)
|
||||
raise e
|
||||
return SearchQueryError(_user_facing_emit_message(d))
|
||||
|
||||
|
||||
|
||||
@@ -82,7 +82,7 @@ class TestEmitErrorRouting:
|
||||
_map_emit_error(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,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
@@ -92,15 +92,21 @@ class TestEmitErrorRouting:
|
||||
WHEN:
|
||||
- _map_emit_error processes it
|
||||
THEN:
|
||||
- It becomes a SearchQueryError, and exactly one ERROR log
|
||||
record is emitted naming the field and the diagnostic kind
|
||||
- Exactly one ERROR log record is emitted naming the field and
|
||||
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
|
||||
with caplog.at_level(logging.ERROR, logger="paperless.search"):
|
||||
error = _map_emit_error(
|
||||
QueryError(_diagnostic(kind, field=FieldRef("asn"))),
|
||||
)
|
||||
assert isinstance(error, SearchQueryError)
|
||||
error = QueryError(_diagnostic(kind, field=FieldRef("asn")))
|
||||
with (
|
||||
caplog.at_level(logging.ERROR, logger="paperless.search"),
|
||||
pytest.raises(QueryError) as excinfo,
|
||||
):
|
||||
_map_emit_error(error)
|
||||
assert excinfo.value is error
|
||||
errors = [r for r in caplog.records if r.levelno == logging.ERROR]
|
||||
assert len(errors) == 1
|
||||
assert "asn" in errors[0].getMessage()
|
||||
@@ -144,7 +150,6 @@ class TestEmitErrorRouting:
|
||||
DiagnosticKind.TEXT_RANGE,
|
||||
DiagnosticKind.PATTERN_TOO_COMPLEX,
|
||||
DiagnosticKind.EXISTS_REQUIRES_FAST,
|
||||
DiagnosticKind.SCHEMA_FIELD_MISSING,
|
||||
],
|
||||
)
|
||||
def test_user_facing_message_never_echoes_library_prose(
|
||||
@@ -154,7 +159,10 @@ class TestEmitErrorRouting:
|
||||
"""
|
||||
GIVEN:
|
||||
- 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:
|
||||
- _map_emit_error processes it
|
||||
THEN:
|
||||
@@ -170,7 +178,6 @@ class TestEmitErrorRouting:
|
||||
DiagnosticKind.TEXT_RANGE,
|
||||
DiagnosticKind.PATTERN_TOO_COMPLEX,
|
||||
DiagnosticKind.EXISTS_REQUIRES_FAST,
|
||||
DiagnosticKind.SCHEMA_FIELD_MISSING,
|
||||
],
|
||||
)
|
||||
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:*``.
|
||||
|
||||
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
|
||||
@@ -81,7 +84,7 @@ class TestJsonExistsIsUserError:
|
||||
|
||||
|
||||
class TestGenuineMisconfigurationStillLogs:
|
||||
def test_schema_field_missing_is_an_error_log(
|
||||
def test_schema_field_missing_is_an_error_log_and_reraised(
|
||||
self,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
@@ -92,9 +95,13 @@ class TestGenuineMisconfigurationStillLogs:
|
||||
WHEN:
|
||||
- _map_emit_error processes it
|
||||
THEN:
|
||||
- It becomes a SearchQueryError and logs exactly one ERROR
|
||||
record naming the diagnostic kind, since this is a real
|
||||
mismatch an operator can fix and keeps the alert
|
||||
- It logs exactly one ERROR record naming the diagnostic kind
|
||||
(a real mismatch an operator can fix, so it keeps the
|
||||
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
|
||||
error = QueryError(
|
||||
@@ -106,9 +113,12 @@ class TestGenuineMisconfigurationStillLogs:
|
||||
field_kind=FieldKind.U64,
|
||||
),
|
||||
)
|
||||
with caplog.at_level(logging.ERROR, logger="paperless.search"):
|
||||
mapped = _map_emit_error(error)
|
||||
assert isinstance(mapped, SearchQueryError)
|
||||
with (
|
||||
caplog.at_level(logging.ERROR, logger="paperless.search"),
|
||||
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]
|
||||
assert len(records) == 1
|
||||
assert kind.name in records[0].getMessage()
|
||||
|
||||
Reference in New Issue
Block a user