Summary
#1491 closed #1452 and fixed the two cases that issue named — a circular container, and a value whose __str__ raises. Both verified fixed on main.
Two other inputs still lose the record entirely. Both defeat the fallback rather than the primary json.dumps, so the guard added by #1491 does not hold, and the comment's promise ("never lose a record") is still stronger than the code.
Verified against origin/main after #1491 merged:
circular emitted=True handleError=False <- fixed by #1491
exploding-__str__ emitted=True handleError=False <- fixed by #1491
oversized-int emitted=False handleError=True <- STILL LOST
exception-with-raising-__str__ emitted=False handleError=True <- STILL LOST
Hole 1 — an oversized int defeats the scalar filter
The fallback keeps values by type:
safe = {k: v for k, v in payload.items()
if isinstance(v, (str, int, float, bool, type(None)))}int is a scalar and passes the filter — but CPython caps int→str conversion at 4300 digits (sys.get_int_max_str_digits()), so a large correlation_id fails the primary json.dumps and then fails the fallback identically, because the filter retained the very value that caused the failure.
logger.info("probe", extra={"request_id": 10**4400})record emitted : False
handleError : True
Reproduction note: build the integer arithmetically. int("9" * 4301) raises during the string→int parse, before the value ever reaches the formatter.
Root cause is the axis, not the predicate. Filtering by type cannot work here; the fallback has to select by provenance. performance_ms and correlation_id are the only caller-supplied values in the payload, so they are the only ones that can carry whatever caused the failure. Rebuilding from the fields the formatter derives from the LogRecord itself — and dropping the enrichments outright — is what actually closes it.
Hole 2 — the detail string is built unguarded
safe["serialization_error"] = f"{type(exc).__name__}: {exc}"{exc} calls str(exc). If the failing object's exception carries its own raising __str__, that raises inside the except handler, and the record is lost again:
class EvilExc(Exception):
def __str__(self): raise RuntimeError("exc str boom")
class Trigger:
def __str__(self): raise EvilExc()
logger.info("probe", extra={"request_id": Trigger()})emitted= False handleError= True
Fix is a nested guard falling back to type(exc).__name__.
Severity
Low, and not attacker-reachable — same reachability as #1452. correlation_id comes from record.request_id and header values (strings); performance_ms is a formatted string. Both holes need a future call site passing an oversized int or an object with a pathological __str__. Filing it because JSON_LOGGING defaults to "true" in production_config.py:72, so this is the production path, and because the code currently claims a guarantee it does not deliver — the exact defect #1452 was about.
Provenance
Found on #1488 (the competing implementation of #1452, closed in favour of #1491). Hole 1 was independently confirmed by CodeRabbit's review there — comment — which called it a blocking fallback failure on that PR's identical first draft. #1488 subsequently fixed both, plus added a constant last-resort tier, but was closed as superseded before those commits were reviewed for merge. The fixes are not on main; only the first draft's approach is.
The working diff is on branch claude/clever-heisenberg-th9bml @ 1fa4f18 if it is useful to cherry-pick — not reopening a PR, since #1452 is closed and #1488 was closed deliberately.
Acceptance criteria
- A record with
extra={"request_id": 10**4400} still reaches the sink, with level authoritative and serialization_error present.
- A record whose failing value raises an exception with its own raising
__str__ still reaches the sink.
- The value that caused the failure is not carried into the fallback record.
- Regression tests for both, failing against the current
main implementation.
- Optional but recommended: a constant last-resort tier, so "never lost" is unconditional rather than an enumeration of anticipated failures. Test it through the real control path (poison a core field such as
process) — asserting on the constant alone is vacuous and passes with the branch deleted.
Summary
#1491 closed #1452 and fixed the two cases that issue named — a circular container, and a value whose
__str__raises. Both verified fixed onmain.Two other inputs still lose the record entirely. Both defeat the fallback rather than the primary
json.dumps, so the guard added by #1491 does not hold, and the comment's promise ("never lose a record") is still stronger than the code.Verified against
origin/mainafter #1491 merged:Hole 1 — an oversized
intdefeats the scalar filterThe fallback keeps values by type:
intis a scalar and passes the filter — but CPython caps int→str conversion at 4300 digits (sys.get_int_max_str_digits()), so a largecorrelation_idfails the primaryjson.dumpsand then fails the fallback identically, because the filter retained the very value that caused the failure.Reproduction note: build the integer arithmetically.
int("9" * 4301)raises during the string→int parse, before the value ever reaches the formatter.Root cause is the axis, not the predicate. Filtering by type cannot work here; the fallback has to select by provenance.
performance_msandcorrelation_idare the only caller-supplied values in the payload, so they are the only ones that can carry whatever caused the failure. Rebuilding from the fields the formatter derives from theLogRecorditself — and dropping the enrichments outright — is what actually closes it.Hole 2 — the detail string is built unguarded
{exc}callsstr(exc). If the failing object's exception carries its own raising__str__, that raises inside theexcepthandler, and the record is lost again:Fix is a nested guard falling back to
type(exc).__name__.Severity
Low, and not attacker-reachable — same reachability as #1452.
correlation_idcomes fromrecord.request_idand header values (strings);performance_msis a formatted string. Both holes need a future call site passing an oversized int or an object with a pathological__str__. Filing it becauseJSON_LOGGINGdefaults to"true"inproduction_config.py:72, so this is the production path, and because the code currently claims a guarantee it does not deliver — the exact defect #1452 was about.Provenance
Found on #1488 (the competing implementation of #1452, closed in favour of #1491). Hole 1 was independently confirmed by CodeRabbit's review there — comment — which called it a blocking fallback failure on that PR's identical first draft. #1488 subsequently fixed both, plus added a constant last-resort tier, but was closed as superseded before those commits were reviewed for merge. The fixes are not on
main; only the first draft's approach is.The working diff is on branch
claude/clever-heisenberg-th9bml@1fa4f18if it is useful to cherry-pick — not reopening a PR, since #1452 is closed and #1488 was closed deliberately.Acceptance criteria
extra={"request_id": 10**4400}still reaches the sink, withlevelauthoritative andserialization_errorpresent.__str__still reaches the sink.mainimplementation.process) — asserting on the constant alone is vacuous and passes with the branch deleted.