fix(core): one lazy AsyncLocalStorage, so a browser bundle can load @ultimat3/core - #256
Conversation
…ultimat3/core
Three modules constructed `new AsyncLocalStorage(...)` at module scope —
`context.ts`, `telemetry.ts` and `impersonate.ts`. A browser bundler stubs
`node:async_hooks` to `{}`, so the emitted chunk carried
var {AsyncLocalStorage} = (() => ({})); var storage = new AsyncLocalStorage;
and threw at MODULE EVALUATION, before any code ran:
TypeError: undefined is not a constructor
Any island importing a ui component hit it — `Button.tsx` -> `Spinner.tsx` ->
`theme/context` -> `errors.ts` -> `@ultimat3/core`. So `@ultimat3/ui`, which its
own manifest calls a "SolidJS design system", could not be put on a client by
the only client bundler the framework has.
`async-context.ts` is the one lazily-constructed storage: `asyncContext(subject)`
returns `{ get, run }` and constructs on first `run()`. Three call sites, one
seam — three copies of the same lazy-construct logic would be three answers to
one question, and the copies are what drift.
Reads degrade, writes refuse. `get()` answers `undefined` in a browser, which is
a definite no rather than an exception: there is no request in flight to miss.
`run()` throws `X_ASYNC_CONTEXT_UNAVAILABLE`, because there the caller asked for
something the runtime provably cannot deliver. The precedent is `solid()` in
`packages/ui/src/theme/solid-adapter.ts`, which returns an inert runtime with no
DOM and throws only when a DOM is present and the registration is missing.
A synchronous save/restore shim was considered as the browser fallback for
`run()` and rejected: it works for sync code and is silently wrong across an
`await` — the same failure-in-the-dangerous-direction that got `jobs.driver`
deleted in 5.0.0. The reasoning is kept in the file so it is not re-proposed.
`impersonate()` gained no refusal of its own. Confirmed by execution, not by
reading: `useContext()` runs first and throws `X_NO_CONTEXT`, so the seam's
refusal is unreachable there. A test pins that ordering.
Server semantics are unchanged: `getStore()` before any `run()` answers
`undefined` whether or not the storage was ever constructed.
The guard is a browser-target build of `packages/core/src/index.ts` that must
evaluate — the barrel is the entry `ui` actually reaches, and its chunk contains
63 of core's 70 non-test source files, so a fourth module-scope construction
anywhere in the package turns it red. It also fails if the bundler ever inlined
the real `node:async_hooks`, so the other assertions cannot pass for the wrong
reason.
before -> packages/ui/src/index.ts THREW: undefined is not a constructor
after -> packages/ui/src/index.ts EVALUATED OK, 205.2 kB
`X_ASYNC_CONTEXT_UNAVAILABLE` is pinned in `scripts/error-map-backlog.ts` rather
than given an HTTP row: it fires only where `node:async_hooks` is stubbed, which
is a browser bundle and never a request being served.
Fixes #244
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KgsU1WBJMAjnLaazvfuJmD
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 55 minutes Limit details: You’ve used the included review currently available. Your 76 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
Comment |
|
CI passed ✓ — semver impact is minor, no breaking changes, and the fix is well-tested. Ready to merge. 🤖 Posted by developerz.ai — the maintainer agent, not a human. |
Fixes #244
The bug
Three modules constructed
new AsyncLocalStorage(...)at module scope:context.ts:75,telemetry.ts:121,impersonate.ts:11. A browser bundler stubsnode:async_hooksto{}, so the emitted chunk carriedand threw at module evaluation, before any code ran:
Any island importing a ui component hit it:
Button.tsx→Spinner.tsx:7→theme/context→errors.ts:4→@ultimat3/core. So@ultimat3/ui— whichpackages/ui/package.json:4calls a "SolidJS design system" — could not be put on a client by the only client bundler the framework has.Scope note: the issue was filed as one site. It was three.
packages/ui/src/errors.ts:4imports the barrel, notcontext.ts, so fixing one site left the barrel still throwing. Measured, not assumed.The fix
async-context.ts(new, 77 LOC) is the one lazily-constructed storage.asyncContext(subject)returns{ get, run }and constructs on firstrun(), probingtypeof AsyncLocalStorage !== 'function'for the bundler stub.Three call sites, one seam. Three copies of the same lazy-construct logic would be three answers to one question (axiom 1), and the copies are what drift.
context.tsdrops from 273 LOC (my first cut, with inline lazy machinery) to 242 — the seam extraction is the whole recovery.The doctrine: reads degrade, writes refuse
get()answersundefinedin a browser — a definite no, not an exception: there is no request in flight to miss.run()throwsX_ASYNC_CONTEXT_UNAVAILABLE, because there the caller asked for something the runtime provably cannot deliver, and runningfnwith no scope moves the failure one frame away where it surfaces asX_NO_CONTEXTblaming the callee.The precedent is
solid()(packages/ui/src/theme/solid-adapter.ts:56-65): inert runtime with no DOM, throw only when a DOM is present and the registration is genuinely missing.A synchronous save/restore shim was considered and rejected. It works for sync code and is silently wrong across an
await— the same failure-in-the-dangerous-direction that gotjobs.driverdeleted in 5.0.0. The reasoning is written into the file so it is not re-proposed in a year.impersonate()gained no refusal of its own. Confirmed by execution, not reading: in the browser barrel it throwsX_NO_CONTEXT, becauseuseContext()atimpersonate.ts:40runs before anything touches the storage. Nothing was added for symmetry; a test pins the ordering so a refactor cannot quietly swap which code a browser caller sees.withSpanthrows rather than degrading to an unparented span. Losing a span's parent is an observability degradation, not a correctness bug, so a silent fallback was arguable — but nothing underpackages/ui/src/callswithSpan/startSpan/currentSpan/impersonate(grepped, zero hits), so throwing costs no real caller, and two doctrines for one question is axiom 1.Server semantics are unchanged.
getStore()before anyrun()answersundefinedwhether or not the storage was ever constructed, so nothing observable moved.The guard
A browser-target build of
packages/core/src/index.tsthat must evaluate. The barrel is the entryuiactually reaches, and its chunk contains 63 of core's 70 non-test source files — the 7 absent areindex.tsitself, three re-export barrels whose targets are all included, and three type-only/fixture files. So a fourth module-scope construction anywhere in the package turns it red.It is also the anti-vacuity guard: if the bundler ever inlined the real
node:async_hooks,runWithContextandwithSpanwould open a scope instead of refusing, and this fails — so the two evaluation assertions above it cannot pass for the wrong reason.Red-then-green, per site:
context.tsreverted to HEAD'new AsyncLocalStorage'telemetry.tsreverted to HEAD'new AsyncLocalStorage2'impersonate.tsreverted to HEAD'new AsyncLocalStorage2'get()→return undefinedcontext.test.tstests redrun()falls through tofn()impersonate()gains its own refusalX_NO_CONTEXT, gotX_ASYNC_CONTEXT_UNAVAILABLEError code
X_ASYNC_CONTEXT_UNAVAILABLE— terminal; no retry ever gains a runtime anAsyncLocalStorage. Row added towiki/Error-Codes.md, title added toCORE_CODE_TITLES, manifest regenerated.Pinned in
scripts/error-map-backlog.tsrather than given an HTTP status row: it fires only wherenode:async_hooksis stubbed, which is a browser bundle and never a request being served.X_NO_CONTEXT, its sibling, is pinned in the same group.bun run scripts/error-map.ts→310 codes in scope, 76 rows, 234 pinned — the status table is closed.Gate
bun test packages/core/— 770 pass, 0 fail, 58 files.boundaries✓ 4006 files.error-render✓ 3890 files.typecheckclean.Full
bun run verifyis red on this machine for an unrelated, pre-existing reason: the dev box runs Bun 1.4.0 (ICU 78) while CI pins 1.3.x (ICU 75), and fourpackages/timetests depend on ICU output. Tracked as #251, fixed separately. Nothing in this PR touchespackages/time; CI on 1.3.x is the authoritative run here.Follow-up
#255 — six more module-scope constructions outside core (
db×3,entity,ai×2). Adopting the seam there needsasyncContextexported fromindex.tsplus aREADME.mdrow, and the enforcement to pair with it is a repo-wide gate refusingnew AsyncLocalStorageoutsideasync-context.ts. Today the rule is enforced forcoreonly, by the barrel test. Stating that plainly rather than claiming a broader guard than exists.🤖 Generated with Claude Code
https://claude.ai/code/session_01KgsU1WBJMAjnLaazvfuJmD
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.