fix(elysia): polyfill enterWith for Cloudflare Workers#395
Conversation
Cloudflare Workers omit native AsyncLocalStorage.enterWith(), which the Elysia integration relies on across lifecycle hooks.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for following the naming conventions! 🙏 |
|
Warning Review limit reached
More reviews will be available in 17 minutes and 12 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a ChangesAsyncLocalStorage enterWith polyfill and Elysia integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The Workers regression test mutated AsyncLocalStorage.prototype and flaked under CI sharding; cover the pattern via a subclass-based unit test instead.
Use object-typed WeakMap keys and explicit run callback signatures so the polyfill passes strict TypeScript and eslint checks.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/evlog/src/elysia/index.ts`:
- Around line 116-118: The `storage.enterWith(logger)` call in the Elysia
request handler and corresponding `storage.enterWith(undefined)` cleanup in
`onAfterResponse` use a module-level singleton storage implementation that does
not properly isolate logger context for concurrent requests on Cloudflare
Workers. The polyfill stores context in a process-global WeakMap instead of
binding to async execution context, causing interleaved requests to leak and
contaminate each other's logger state. Replace the `enterWith()` pattern with
`storage.run()` to wrap the relevant request handling logic, since the polyfill
properly delegates `run()` to the native implementation which Workers supports.
Additionally, add a concurrency regression test that simulates multiple
simultaneous interleaved requests to verify logger context isolation and prevent
this contamination issue from reoccurring.
In `@packages/evlog/src/shared/asyncStorageScope.ts`:
- Around line 11-42: The fallbackStores WeakMap in
installAsyncLocalStorageEnterWithPolyfill() is keyed only by the ALS instance,
causing all concurrent async contexts to share the same store value instead of
isolating per execution context as native enterWith() does. This results in
cross-request context bleed when multiple requests call enterWith() without
run() wrappers. Fix this by either modifying fallbackStores to be keyed by
execution context (e.g., using a Map that tracks execution state alongside the
ALS instance in enterWith() and getStore()), or by wrapping all Elysia
integration calls to storage.enterWith() in run() frames per request.
Additionally, correct the JSDoc comment on lines 11-12 to accurately reflect
that this polyfill does not provide the same concurrency semantics as native
enterWith() and only guarantees isolation within a single run() invocation.
In `@packages/evlog/test/frameworks/elysia.test.ts`:
- Around line 394-419: The test captures and restores only the `enterWith`
method from AsyncLocalStorage.prototype, but the
`installAsyncLocalStorageEnterWithPolyfill()` function patches three methods:
`enterWith`, `getStore`, and `run`. To prevent state leakage between tests,
capture all three methods at the beginning (before calling
installAsyncLocalStorageEnterWithPolyfill) by destructuring `getStore` and `run`
alongside `enterWith` from AsyncLocalStorage.prototype, then restore all three
methods in the finally block using Object.defineProperty calls for each method,
following the pattern demonstrated in asyncStorageScope.test.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0c933f74-e654-4a34-9897-18776f20fa0f
📒 Files selected for processing (5)
.changeset/fix-elysia-workers-asynclocalstorage.mdpackages/evlog/src/elysia/index.tspackages/evlog/src/shared/asyncStorageScope.tspackages/evlog/test/frameworks/elysia.test.tspackages/evlog/test/shared/asyncStorageScope.test.ts
commit: |
Use native AsyncLocalStorage helpers, document Workers concurrency limits honestly, and add a derive-based concurrency regression test.
|
@zAlweNy26 Can you try the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/evlog/src/shared/asyncStorageScope.ts`:
- Around line 107-109: The function installAsyncLocalStorageEnterWithPolyfill is
patching AsyncLocalStorage.prototype globally, which affects all
AsyncLocalStorage instances in the isolate and can break other dependencies.
Instead of calling this function to patch the global prototype, locate the
module-level storage instance used by evlog and patch that specific instance
directly using patchAsyncLocalStorageEnterWith. Remove the global prototype
patching approach and apply the patch only to the evlog storage object (likely
created in asyncStorageScope.ts or passed from elysia/index.ts) right after it
is instantiated. This way, only evlog's storage instance gets the custom
enterWith behavior without affecting other ALS instances.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3d0d6730-307a-4a16-b3cf-9ebcccbbbc31
📒 Files selected for processing (4)
.changeset/fix-elysia-workers-asynclocalstorage.mdpackages/evlog/src/elysia/index.tspackages/evlog/src/shared/asyncStorageScope.tspackages/evlog/test/shared/asyncStorageScope.test.ts
Scope the enterWith polyfill to evlog's storage object instead of mutating AsyncLocalStorage.prototype globally.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/evlog/test/shared/asyncStorageScope.test.ts (1)
16-24: 🎯 Functional Correctness | 🔵 TrivialAdd a test case for a Workers-style throwing
enterWithshim to improve defensive coverage.The current test helper only models an absent
enterWith(set toundefined). The polyfill's check on line 10 of the source (typeof storage.enterWith === 'function') means that ifenterWithexists as a function—even one that throws—the patch will skip early and not provide a fallback. Adding a test for the throwing case ensures the patch handles that scenario defensively, matching potential edge cases in runtime implementations.Regression test shape
+ it('patches a Workers-style throwing enterWith shim', () => { + class LocalAsyncLocalStorage extends AsyncLocalStorage<string> {} + Object.defineProperty(LocalAsyncLocalStorage.prototype, 'enterWith', { + configurable: true, + writable: true, + value() { + throw new Error('asyncLocalStorage.enterWith() is not implemented') + }, + }) + + const storage = new LocalAsyncLocalStorage() + patchAsyncLocalStorageEnterWith(storage) + + expect(() => storage.enterWith('evlog')).not.toThrow() + expect(storage.getStore()).toBe('evlog') + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evlog/test/shared/asyncStorageScope.test.ts` around lines 16 - 24, The current test helper createWorkersLikeStorage only models an absent enterWith (set to undefined), but the polyfill will skip patching if enterWith exists as any function (per the typeof check). Add a new test case that creates an AsyncLocalStorage where enterWith is defined as a function that throws an error instead of being undefined, then verify that the patchAsyncLocalStorageEnterWith function handles this throwing scenario defensively without crashing. This ensures coverage of the edge case where enterWith exists but fails at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/evlog/src/shared/asyncStorageScope.ts`:
- Line 46: The early-return check for
supportsAsyncLocalStorageEnterWith(storage) at line 46 does not account for
environments like Cloudflare Workers where enterWith may be a callable that
throws an error rather than being undefined. Wrap the early-return condition in
a try-catch block: if supportsAsyncLocalStorageEnterWith(storage) executes
successfully, return early as before; if it throws an exception, catch the error
and skip the return so the code falls through to the polyfill fallback instead.
This ensures that mock or non-functional implementations of enterWith will not
bypass the polyfill and cause bindAsyncLocalStorage() to fail.
---
Outside diff comments:
In `@packages/evlog/test/shared/asyncStorageScope.test.ts`:
- Around line 16-24: The current test helper createWorkersLikeStorage only
models an absent enterWith (set to undefined), but the polyfill will skip
patching if enterWith exists as any function (per the typeof check). Add a new
test case that creates an AsyncLocalStorage where enterWith is defined as a
function that throws an error instead of being undefined, then verify that the
patchAsyncLocalStorageEnterWith function handles this throwing scenario
defensively without crashing. This ensures coverage of the edge case where
enterWith exists but fails at runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7d4b1078-8208-4449-a1a8-70dc351cb342
📒 Files selected for processing (3)
packages/evlog/src/elysia/index.tspackages/evlog/src/shared/asyncStorageScope.tspackages/evlog/test/shared/asyncStorageScope.test.ts
Cloudflare Workers expose enterWith but throw when called; detect that at probe time so the polyfill still applies on those runtimes.
1 participant
Summary
AsyncLocalStorage.enterWith()polyfill when the runtime omits it (Cloudflare Workers)enterWith()souseLogger()works across async boundaries between lifecycle hooksCloses #394
Test plan
pnpm run lintpnpm run typecheckpnpm run testwrangler devwith Elysia +evlog()no longer throwsenterWith() is not implementedSummary by CodeRabbit
wrangler dev-style flows).useLogger()guidance to clarify availability across async boundaries.AsyncLocalStoragebehavior and concurrency isolation between interleaved requests.