Repository navigation
fix(objectql): report a refused write at warn, not error — the caller was already told - #17211
Conversation
`insert`, `update` and `delete` each end their catch with `throw e` and logged
the failure at ERROR one statement earlier. AGENTS.md → Degradation log levels
names that shape: "a failure handed to the CALLER is not a degradation at all
… Do not bolt a `logger.error` onto such a site."
The diagnosis two earlier rulings shaped survives the move. `warn(message,
meta?)` has no `Error` slot, and `Error.message`/`stack` are non-enumerable, so
handing the error over as meta would have serialised `{}`. The doors instead
build the `{ error: { message, stack } }` bag `ObjectLogger.write()` builds from
the slot, keeping #8682's redaction and #14095's choice of the driver error over
the envelope. The rendered line is byte-identical apart from the level word.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wtfW1ZxGnP1XKGc9uZVms
…sert-failure-log-level
📓 Docs Drift CheckThis PR changes 1 package(s): 12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0bdee5228faa9edcd4bb14d5fc0911654e8c8d45 && git checkout 0bdee5228faa9edcd4bb14d5fc0911654e8c8d45
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a95b0e93d048d96736a5ab678642416b6277c42 2cb8dfe706822dd3762b3534304dce5a3cd0c53f && git checkout -B drift-repro 5a95b0e93d048d96736a5ab678642416b6277c42 && git merge --no-ff 2cb8dfe706822dd3762b3534304dce5a3cd0c53f
node scripts/docs-audit/affected-docs.mjs --json 5a95b0e93d048d96736a5ab678642416b6277c42
|
…7052) Running the whole package rather than the message-matched files found one consumer a message grep could not see, plus two rig defects of my own: - `engine-delete-restricted-locale.test.ts` — #7307's pin that the delete door's `developerMessage` reaches the SERVER LOG. It monkey-patches the logger method and reads the meta by ARGUMENT POSITION, so it never mentions `Delete operation failed` and no grep for that message could find it. Now captures both `warn` and `error`, taking meta from the right position for each, so it stays a statement about the log rather than about one method. - `engine-strict-readonly-warning-truthful.test.ts` — `warns` collected every warn-level message, so the door's entry made each strip assertion count 2. The message isolation this suite already performed in `stripLineLevels()` now backs `warns` too, which is where the strip assertions actually read. - `engine-write-failure-log-level.test.ts` — the delete door was driven with an unrecognised `id` option, so the engine refused the call before reaching the driver and the door logged nothing. Addressed through `where`, which is what #4371's refusal message names. Full suite: 289/289 files, 4865/4865 tests. typecheck: OK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wtfW1ZxGnP1XKGc9uZVms
…sert-failure-log-level
Fixes #17052
Clause-②: no — this PR puts no new key on any published payload. It narrows what the three write doors claim at
error;writeFailureLogMeta()is module-local and unexported, no name was added to any repo-wide list, and no schema, error code or accept set moved. Re-declared from the DELIVERED diff by the reviewing seat, not inherited from the claim. Thepatchgrading below therefore stands.insert,updateanddeleteeach end theircatchwiththrow e, and loggedthe failure at ERROR one statement earlier. AGENTS.md → Degradation log levels
names that exact shape and forbids it: "a failure handed to the CALLER is not
a degradation at all … Do not bolt a
logger.erroronto such a site." Therequester is told, on every path, by the loudest means available. The three
doors now report at
warn, carrying the same diagnosis.⭐ Zone 2 first: the premise, verified before anything was written
The ruling was hung on "the propagation checker can structurally prove every
path out of these catches delivers". Measured, both halves:
The substance holds — every path delivers. Each catch body's only exit is
the unconditional
throw e; there is no branch, no early return, no loop. Thechecker's own model agrees: in
catchDeliversFailure(),analyzeStmton aThrowStatementreturns{ delivered: true, terminates: true }under thecomment "A rethrow IS delivery — the loudest kind." So the fork the dispatch
told me to stop for — some path exits without telling the caller — did not
occur, and shape 1 lands.
The mechanism named in the ruling is unavailable, and that is measured, not
argued.
FAILURE_PROPAGATION_SITEScannot be used here. I probed it ratherthan reasoning about it — added
packages/objectql/src/engine.ts::insertto themap, ran the gate, restored (restore verified by
git hash-objectagainst theHEAD blob,
87a4b61… == 87a4b61…):Two independent structural reasons:
analyzeSourceFile's step 1(
collectGuardedCalls) returns empty — the raw ObjectQL verbs aredeliberately not in
DURABILITY_CRITICAL_CALLEES, and the checker's ownheader says why (The durability log-level gate cannot see the
catch { return null; }seeder family — 15 files outside #12923's five, and neither widening path is cheap #12981: declaring them repo-wide drags in 36 quietdegradations).
--listconfirms it: of the log-level rule's 29 seams theonly
engine.tsentry issyncSchema()@14484.catchDeliversFailure()requiresevidence— a declared propagation call. A barethrow eleavesevidenceundefined, so the function returnsundefinedand the site isexcused by the rethrow limb (
propagatesAlways), never by a declaration.⇒ A declaration here is stale by construction and turns
pnpm check:durability-log-levelred. ⛔ I did not weaken the checker to make it fit;the correct reading is that the checker already proves delivery for this shape
by the route it reserves for rethrows.
pnpm check:durability-log-levelisgreen on the delivered tree (29 seams, 63 self-test cases).
⛔ How I verified the two earlier rulings survived (#8682, #14095)
This is the part that nearly went wrong, and the trap is worth naming: the
Loggercontract gives anErrorslot toerror/fatalonly.warn(message, meta?)has two parameters. AndError.message/Error.stackarenon-enumerable, so handing the error to
warnas meta does not merely losethe slot — it serialises
{}and destroys the diagnosis one level down, whichis precisely the loss #14095 refused.
So the doors build the bag
ObjectLogger.write()builds from the slot(
{ …meta, error: { message, stack } }), via a module-localwriteFailureLogMeta(). Measured against the real platform logger — before(old
errorcall) vs after (newwarncall), plus the naive move as a negativecontrol:
redactBoundStatement(...), and it is that result'smessage/stackthat areread. The suite that pins it (
driver-fault-redaction.test.ts) still assertsthe failing column is named, and that neither field carries
SECRET,DESCRIPTIONorinsert into.isUniqueViolationErroris not reachable from an app #14095 — the entry still takes thecause, not the envelope: the choiceof which value to hand in is unchanged at all three call sites, so MySQL's
for key 'crm_account.secret_note'and the driver's own frames still arrive.Its suite (
engine-update-duplicate-record.test.ts) reads the same two fieldsit always did, now out of
meta.error.The sweep the card marked NOT MEASURED
AST matcher over
packages/objectql/src(145 files carrying acatch),following same-file helper calls one frame — the same visibility
collectLoggedLevels()has — and discriminating by path: an error log countsonly when it is on the same path as the rethrow.
engine.ts:9710→Find operation failed(viareportFindFailure)engine.ts:11010→Insert operation failed⭐ positive controlengine.ts:12382→Update operation failed⭐ positive controlengine.ts:13832→Delete operation failedhook-wrappers.ts:418→ logs at ERROR only on the branch that suppresses (onError:'log', thenreturn); its rethrow branch logs nothingPositive control: the matcher finds both sites the card established, by name.
Negative control:
hook-wrappers.tsis a real ERROR-and-rethrow-in-one-catchand is still classified out, so the path limb is exercised rather than vacuous —
and it is correct as it stands, because on the branch that logs, the caller is
not told.
Repaired: 3 of 4 —
insert,update,delete. The write doors are oneshape in one file, and
deleteis the same defect the card describes.Scoped out and filed:
find. Its level was set deliberately by a separateruling (
reportFindFailure's docblock weighs the driver's own survivingwarnagainst a second line) whose scope note explicitly reasoned about the write
verbs. Moving it is a decision, not a repair, so it is a card rather than a rider
— see
out_of_scope_findings. Both stale prose rulings that would otherwise havebeen left contradicting the code are corrected here:
reportFindFailure's"
insert/update/deletekeep their unconditionalerror", anddriver-fault-redaction.ts's specimen line and its "not to lower the level"sentence (whose ⛔ was about this exposure's remedy and stays).
Consumers surveyed before moving the level
Every mention of the three messages, repo-wide (29 hits, 18 files), classified:
Depends on the level (4):
scripts/publish-smoke.sh— ⭐ the consumer that made this a release problem.SMOKE_ERROR_LOG_PATTERNfails a boot on any error-level line. Its own headerrules the direction this change relies on: "⛔ The severity is NOT the
predicate. Widening the error scan to WARN is the obvious repair and it is the
wrong one: warn is where this repo deliberately puts functional degradation …
so a healthy boot HAS warn lines". So
warnis the level this gate is builtto tolerate, and no gate change is owed.
engine-strict-readonly-warning-truthful.test.ts(⭐ positive control — the pinthe dispatch named),
engine-find-missing-table-log-level.test.ts,driver-fault-redaction.test.ts,engine-update-duplicate-record.test.ts.All four moved, not deleted — each now asserts the level the door takes,
and three gained an assertion that nothing is emitted at
errorany more, sothe move cannot be read as a copy.
Does not depend on the level (7 runtime prose references):
plugin-security(×4),
cli/src/utils/schema-migration-plugins.ts, and two test comments allpoint operators at "the query engine's 'Insert operation failed' / 'Update
operation failed' entries" — they name the message, never the level, so they
stay true. Verified by reading each string, not by grepping for a level word.
Level choice:
warn, notdebugThe card allowed either.
debugis refused by evidence, not taste:engine-find-missing-table-log-level.test.tsalready pinned that this line mustnot land at
debug(itstoHaveLength(0)assertion is untouched by this PRand still passes), and the platform default level is
info— atdebugthediagnosis #8682 and #14095 fought to keep would be invisible in production,
which Zone 1 rule 4 forbids.
warnkeeps it on by default and off the channelpublish-smokereads.Gates
62 derived families from
dispatch-gates.mjs --commands, exit codes captured todisk before any pipe, reconciled with
--ran:✓ dispatch-gates --ran: 62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED.check:durability-log-level(the neighbouring gate thecard asks about — it stays green, and its rule was never the one firing here:
it fires on a catch logging below error without rethrowing, the opposite
direction),
check:nul-bytes,check:published-files,check:doc-authoring,check:test-source-alias,check:type-check-coverage.pnpm --filter '@objectstack/objectql...' build, three of them are green:check:dts-closure(78/78 declaration files across 15 built packages),check:lean-entry-closure,check:sourcemap-no-sources-content.check:dual-build-cjs-loads(47 more packages unbuilt) and
check:type-check-debt(its--re-measurehalfneeds 17 more dependency closures, and it refuses to measure a different world).
Recorded NOT MEASURED — neither pass nor failure — and left to CI, which builds
the farm.
Tests — measured
pnpm --filter @objectstack/objectql test→ 289/289 files, 4865/4865 testspassing.
pnpm --filter @objectstack/objectql typecheck→ OK.was held continuously by another seat's run (
issue-8772/final.sh, pid 5749) for~37 minutes, and four calls returned
VERDICT queue-timeout (exit 99) · never acquiredbefore the lock freed. The place was kept viaOS_VERIFY_LOCK_SLOTthroughout, and nothing was recorded as a red on the strength of an unacquired
lock.
Running the WHOLE package rather than the message-matched files paid for
itself — it found one consumer no grep could have reached, and two defects in
my own rig:
engine-delete-restricted-locale.test.ts— DELETE_RESTRICTED (409) message is shown verbatim to end users: English-only, leaks API names, and contains developer-facing advice #7307's pin that the delete door'sdeveloperMessagereaches the server log. It monkey-patches the logger methodand reads the meta by argument position, so it never mentions
Delete operation failedand the message survey could not see it.warn(message, meta)has no
Errorslot, so the meta moved from the third argument to the second.It now captures both levels and takes meta from the right position for each,
which makes it a statement about the log rather than about one method.
engine-strict-readonly-warning-truthful.test.ts—warnscollected everywarn-level message, so the door's entry turned each strip assertion into a
count of 2. The message isolation this suite already performed in
stripLineLevels()now backswarnstoo, which is where the strip assertionsactually read. (This is the accessor half of the isolation its docblock always
described.)
engine-write-failure-log-level.test.ts— my new rig drove the delete door withan unrecognised
idoption, so the engine refused the call before the driverand the door logged nothing at all. Addressed through
where, which is what[P2] A direct engine call silently drops
sort/select/skip/populate— declared query contract, zero enforcement #4371's own refusal message names.Changeset
.changeset/engine-write-failure-log-level-warn.md,patch— publishedbehaviour does move (the level is what an operator greps, and
publish-smoke.shreads it structurally), soskip-changesetwould have beenwrong. It states the FROM → TO for anyone grepping or alerting on these lines.
维护者速读(草稿)
改了什么 —
packages/objectql的三个写入门(insert/update/delete)在把失败抛回调用方之前打的那条日志,级别从error降到warn;内容一个字节没动。为什么改 — AGENTS.md 明文规定:失败已经交给调用方的
catch,不算降级,不该挂logger.error。代价是实测过的:全新项目第一次启动时,@better-auth/oauth-provider用 UNIQUE 约束做竞态保护(它自己的文档就是这么写的),我们却在它 catch 之前先打了一条 ERROR,连续六次把publish-smoke / packed-tarballs刷红,而那个候选版本的探针全绿。风险与代价(含回滚) — 风险集中在一处:
warn没有Error参数位,而Error的message/stack不可枚举,直接把错误对象塞进 meta 会静默变成{},#8682 和 #14095 两轮裁决保下来的诊断就没了。所以按ObjectLogger自己的拼法重建了那个字段,并用真实 logger 实测:除级别词外整行字节一致(负对照见上)。回滚成本低——单包单文件,三处调用点加一个模块内私有函数。已发布面只动了日志级别,changeset 里写清了 FROM → TO。席位意见 — (留空)
你要做的 — 确认
warn是这条线的终局级别(卡片允许warn或debug,我按已有 pin 与默认日志级别选了warn);以及find门的同类问题我按裁决另立卡而没有顺手改,请确认这个边界。🤖 Generated with Claude Code
https://claude.ai/code/session_019wtfW1ZxGnP1XKGc9uZVms