Uh oh!
There was an error while loading. Please reload this page.
fix(core): cache resolveLocalizationContext across requests to stop repeated sys_setting read noise - #10301
Conversation
A fresh environment's sys_setting table doesn't exist yet, so every authenticated request re-issued the same localization read, and every one failed the same way — driver-sql's backendStatementFault logs a [sql-driver] DATABASE_ERROR warning on every failed read, burying real errors between the noise. resolveLocalizationContext now memoizes its result (including the missing-table fallback to UTC/en-US) for 30s per (ql, tenantId, userId), mirroring the TTL cache plugin-audit/audit-writers.ts already applies to this same read. Functional behavior is unchanged; only the repeated per-request query is eliminated.
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 23 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 7bd5eae805c96a7482a84666faf08c0de3fbb792 && git checkout 7bd5eae805c96a7482a84666faf08c0de3fbb792
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 736cfb14b813f0895ca90c3c371ff00d29cd977c 34e7bade970648290cc245367ed5080d063b5beb && git checkout -B drift-repro 736cfb14b813f0895ca90c3c371ff00d29cd977c && git merge --no-ff 34e7bade970648290cc245367ed5080d063b5beb
node scripts/docs-audit/affected-docs.mjs --json 736cfb14b813f0895ca90c3c371ff00d29cd977c
|
…ly (#10221) CI caught the first version's regression: it cached every outcome (including a successful read) for 30s, which broke packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts (#1982/#2018) — that dogfood test writes a new org timezone via the real settings route and expects the very next analytics read to bucket under it. Narrow the cache to memoize ONLY the case where the underlying sys_setting read genuinely throws (a backend fault, e.g. "no such table") — the case that actually produces the log spam #10221 reports. A successful read, including a legitimate empty result, is never cached and always re-reads, so a settings write is visible on the very next call, matching pre-existing behavior. Functional fallback (UTC/en-US on failure) is unchanged. Added two tests pinning the never-cache-a-success guarantee directly (value change and first-write-after-empty, both visible on the next call with no TTL advance), and updated the existing failure-path tests' description accordingly.
check:query-options-erasure caught it: the previous commit's inlined ql.find() call (added to observe read failure for the #10221 cache) cast its options literal to any, a NEW counted erasure site distinct from tryFind's existing grandfathered one. ql is already typed any, so the cast was redundant — tsc doesn't need it, and dropping it restores the ratchet to its pre-existing count. No behavior change.
Uh oh!
There was an error while loading. Please reload this page.
Fixes#10221
Root cause
The reproduction's query shape (
namespace = 'localization',key in ('timezone', 'locale', 'currency'),scope = 'tenant') is issued byresolveLocalizationContextinpackages/core/src/security/resolve-authz-context.ts, called once per authenticated request frompackages/rest/src/rest-server.ts(and similarly frompackages/runtime/src/security/resolve-execution-context.ts). On a fresh environmentsys_settinghasn't been created/migrated/written yet, so this read fails identically on every request, anddriver-sql'sbackendStatementFault(packages/drivers/driver-sql/src/sql-driver.ts) logs a[sql-driver] DATABASE_ERRORwarning on every failed read — the same line, once per request, burying real errors in between.tryFind(the local helperresolveLocalizationContextalready used) already catches the failure and falls back to{ timezone: 'UTC', locale: 'en-US' }— the functional behavior described in the issue ("读不到就回退默认 locale") was already correct.#2409had already collapsed one request's THREE per-key reads into one batched query. What was missing was cross-request de-duplication: the same query re-ran every request forever, with no cache and no bootstrap.Fix (remedy 3 — in-process cache, narrowed to the FAILED outcome only)
resolveLocalizationContextnow memoizes ONLY the case where the underlying read genuinely fails (a backend fault — e.g. "no such table") for 30s, keyed on(ql instance, tenantId, userId). A successful read — including a legitimate "nothing configured yet" empty result — is never cached; the next call always re-reads.qlengine instance first means two environments/tenants sharing one process never share a cached outcome, and the cache is naturally scoped/GC'd per environment (WeakMap).sys_settingexists (migration lands), the failure stops happening and nothing gets cached from then on — no restart required, no explicit invalidation needed.packages/plugins/plugin-audit/src/audit-writers.ts(resolveWriteLocale) — safe there because audit-trail enrichment is best-effort, so a stale locale in a log line for up to 30s is invisible. It is not safe for this function's other callers:@objectstack/restreads the org timezone on every analytics query, andpackages/qa/dogfood/test/analytics-timezone.dogfood.test.ts(the ADR-0053 Phase 2 · Slice 5: timezone-aware analytics date bucketing #1982/fix(analytics): make organization timezone drive date-dimension bucketing (#1982) #2018 golden regression) writes a new org timezone via the real settings route and asserts the very next analytics read buckets under it. Analytics date-bucketing is declared, tested behavior — it cannot tolerate the staleness the audit writer's best-effort enrichment can.Why not the other two remedies
backendStatementFault(packages/drivers/driver-sql) is the shared terminal for every dialect read failure across every object — not specific tosys_settingor to "missing table". Its current generic-warn design is documented as deliberate (see its docblock: "table never provisioned" is one of several conditions intentionally not specialized). Teaching it to recognize and downgrade a specific table/condition would either weaken it for every other object's real failures or require per-call opt-in plumbing that doesn't exist today — a much larger, cross-cutting change for a caller-specific noise problem.sys_settinginto first-boot migration:sys_settingis a normal platform object (packages/platform-objects/src/system/sys-setting.object.ts); there's no existing "eagerly migrate this table on env boot before any read" mechanism to hook into for one object, and forcing table creation earlier doesn't stop a future miss (e.g. a slow migration, another platform table that hasn't landed yet) from producing the same noise. The cache addresses the general "repeated failing per-request read" shape, not just this one table.The cache approach also has direct prior art already applied to this exact function's exact use case elsewhere in the codebase (see above), which is the strongest signal for "matches how the surrounding code already handles this."
Patch round (CI feedback)
CI's Dogfood Regression Gate failed on
analytics-timezone.dogfood.test.ts's"shifts the bucket to the previous day under America/Los_Angeles"case: the first version of this fix cached every read outcome (not just failures) for 30s, so the test's org-timezone write viaPUT /api/settings/localizationwas invisible to the very next analytics query until the cache expired.Fix: narrowed the cache to memoize only the case where the read itself throws (the case that actually produces #10221's log spam), never a successful or legitimately-empty read. Also fixed a
check:query-options-erasure(#4918) hit the first patch introduced — the inlinedsys_settingread (added so a genuine failure is visible to the caching layer) had its own redundantas anyon the options literal, a newly-counted erasure site; removed it sinceqlis already typedanyand the cast added nothing.Added two unit tests pinning the never-cache-a-success guarantee directly at the
resolveLocalizationContextboundary (a value change and a first-write-after-empty-result, both immediately visible with no TTL advance) — the fastest reproduction of the dogfood scenario's shape without booting the full stack (see Test plan; the dogfood package's own rig needs a from-scratch build of ~26@objectstack/*packages, judged too heavy for this container's local budget and left to CI).Changes
packages/core/src/security/resolve-authz-context.ts:resolveLocalizationContextnow wraps a failure-onlyWeakMap<ql, Map<tenantId|userId, {value, expiresAt}>>30s cache around the (unchanged) resolution logic, which now also reports whether the underlying read failed.packages/core/src/security/resolve-authz-context.test.ts:describe('resolveLocalizationContext — failure-only cross-request cache (#10221)')— asserts a failing read is cached and expires correctly, cache entries are isolated per tenant/qlinstance, and — the dogfood guard — a successful read (value change or first write after an empty result) is never cached and is visible on the very next call..changeset/localization-context-ttl-cache.md: patch changeset for@objectstack/core, updated for the narrowed design.Test plan
At
34e7bade9(this branch's head, patch round applied):pnpm --filter '@objectstack/core^...' build(dependency closure:@objectstack/spec,@objectstack/metadata-core) — clean.pnpm --filter '@objectstack/core' build— clean (@objectstack/corehas no dedicatedtypecheckscript; itsbuildruns tsup's DTS pass, which is a full TS type-check and completed with no errors).pnpm --filter '@objectstack/core' test— 875/875 passed (36 files), including the 6 new/updated cache tests (failure-caching + the two never-cache-a-success guards) and the unchanged#2409batching tests.node scripts/pm/dispatch-gates.mjs(no paths — derives the changed set from git, off merge-basece300c8d6) and every gate it named that doesn't require a full-monorepo build:check:authz-resolver,check:cross-package-test-inputs,check:kernel-hook-pairs,check:slot-lookup,check:test-source-alias,check:query-options-erasure(caught & fixed the erasure regression above),check:engine-double-contract,check:where-matcher,check-adr-0087-registration.mjs,check-changeset-no-major.mjs,check-empty-changeset.mjs,check:type-check-coverage,scripts/docs-audit/check-affected-docs.mjs— all green.node scripts/check-nul-bytes.mjs— clean.@objectstack/*dependency closure):pnpm --filter @objectstack/dogfood test. Relying on CI's Dogfood Regression Gate; the unit tests above reproduce the exact failure shape at theresolveLocalizationContextboundary the dogfood test exercises end-to-end.Two gates'
--self-testsub-mode failed; both are pre-existing/environmental and unrelated to this diff — neither script was touched here, and both failures are in the script's own internal self-consistency fixtures, not in scanning this diff's files (filed as #10303):check:changeset-gate-self-tests→check-adr-0087-registration.mjs --self-testfails 3I2cases (exit-code/verdict reporting from a spawned subprocess). The real invocation (node scripts/check-adr-0087-registration.mjs, no--self-test) against this diff passes cleanly, as doescheck-changeset-no-major.mjsandcheck-empty-changeset.mjsrun directly.check:objectui-changeset→objectui-changeset-digest.mjs --self-testthrowsENOENTreading its own generated temp fixture; this gate has no non-self-test mode to fall back to.Declared narrowing:
check:type-check-debt(convention-triggered by the new test file) needs the full workspace built (pnpm exec turbo run build --filter=./packages/* --filter=./packages/*/*) before it will even run — a full-farm build this repo's own local-verification convention says not to run locally (CI runs the farm regardless). Not run here; left to CI.Environment note: this container's
bashis macOS's stock 3.2.57 and has neither/procnorflock, soscripts/pm/os-verify-lock.shcannot function here (confirmed: even after building bash 5.2 into an isolated prefix to clear themapfile/EPOCHSECONDSsymptom already tracked in #10289, the queue never reaches head becauseticket_alive()needs/procandlock_is_held()needsflock, neither present on Darwin — detail posted as a comment on #10289). All commands above were run directly, without the lock, the same workaround #10219 used for the same blocker.Generated by Claude Code