fix(util): detect Cloudflare Workers in shouldUseGlobalFetchAndWebSocket#11456
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughThis PR enhances the Changes
Possibly related issues
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip Migrating from UI to YAML configuration.Use the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/util/__tests__/runtime.test.ts`:
- Around line 16-23: The test mutates globals with delete which
vi.unstubAllGlobals() won't restore; update the test "GIVEN browser env without
fetch or WebSocket THEN returns false" to avoid direct deletion by using
vi.stubGlobal('fetch', undefined) and vi.stubGlobal('WebSocket', undefined) (or
alternatively capture original descriptors for globalThis.fetch and
globalThis.WebSocket and restore them in a try/finally), then assert
shouldUseGlobalFetchAndWebSocket() returns false and ensure cleanup/un-stubbing
occurs so other tests are not affected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7fe9722e-9dbb-4d35-8416-f382e808b8f8
📒 Files selected for processing (2)
packages/util/__tests__/runtime.test.tspackages/util/src/functions/runtime.ts
📜 Review details
🔇 Additional comments (2)
packages/util/src/functions/runtime.ts (1)
9-15: Cloudflare Workers short-circuit is correctly prioritized.This check is in the right place (before the
process.versionsruntime branch) and correctly prevents Workers +nodejs_compatfrom falling into the Node.js path. It aligns with how runtime selection is consumed inpackages/ws/src/ws/WebSocketShard.ts:87-89andpackages/rest/src/index.ts:7.packages/util/__tests__/runtime.test.ts (1)
25-44: Runtime-branch coverage is solid for the new behavior.The Workers (
WebSocketPair) case plus Node, Deno, and Bun paths are clearly asserted and match the updated runtime detection contract.
40d5344 to
cfab41c
Compare
Closes #11455
Problem
shouldUseGlobalFetchAndWebSocket()returnsfalseon Cloudflare Workers withnodejs_compat, causingclient.login()to hang indefinitely.Workers with
nodejs_compatpolyfillsglobalThis.processincludingprocess.versions.node. The function seesprocess, checks fordeno/bunin versions, finds neither, and returnsfalse(defaulting to thewsnpm package (TCP sockets)). Workers doesn't support TCP sockets, so the gateway connection hangs forever with no error.Fix
Check for
globalThis.WebSocketPair(a Workers-only global) before the Deno/Bun branch. This is the same detection approach already used bygetUserAgentAppendix()in the same file.Workers natively supports both
globalThis.WebSocketandglobalThis.fetch, so returningtrueis correct for both consumers (@discordjs/wsand@discordjs/rest).Changes
packages/util/src/functions/runtime.ts— addedWebSocketPaircheck (8 lines)packages/util/__tests__/runtime.test.ts— new file, 6 tests covering every branch