fix(testing): fix concurrent Playwright workers timing out on signIn()#8173
Open
jacekradko wants to merge 12 commits intomainfrom
Open
fix(testing): fix concurrent Playwright workers timing out on signIn()#8173jacekradko wants to merge 12 commits intomainfrom
jacekradko wants to merge 12 commits intomainfrom
Conversation
…t Playwright workers
🦋 Changeset detectedLatest commit: 12e7473 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA changeset was added for 🚥 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/testing/src/playwright/setupClerkTestingToken.ts`:
- Around line 72-91: The retry logic currently lets a final retryable response
fall through and be parsed/fulfilled as an error; modify the branch around
RETRYABLE_STATUS_CODES so that when RETRYABLE_STATUS_CODES.has(status) is true
you check attempt < MAX_ROUTE_RETRIES to back off and continue, but if attempt
>= MAX_ROUTE_RETRIES you immediately fulfill the route with the raw response and
return (e.g., await route.fulfill({ response }); return) instead of calling
response.json() and running the captcha_bypass overrides; adjust the if block in
setupClerkTestingToken.ts to explicitly handle the exhausted-retries case before
parsing JSON.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: f02acff7-c751-4d3a-a5a5-d73817f707d2
📒 Files selected for processing (2)
.changeset/fix-testing-concurrent-workers.mdpackages/testing/src/playwright/setupClerkTestingToken.ts
Contributor
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/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts`:
- Around line 17-18: The test declares fulfilled with json typed as unknown
which prevents property access in assertions; define a concrete shape or cast
before use: create a type alias (e.g., MockFulfilled = { response?: { client?:
unknown }; json?: { response?: { client?: unknown }; client?: unknown } }) and
change the fulfilled declaration to const fulfilled: MockFulfilled[] = []; or,
if you prefer minimal change, add narrow/cast at assertion sites when accessing
fulfilled[i].json (e.g., const j = fulfilled[i].json as { response?: { client?:
unknown } } before using j.response or j.response.client) so TypeScript accepts
the property accesses in the tests.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1daf02a9-4752-4341-b0bf-64dad5d06d27
📒 Files selected for processing (4)
packages/testing/package.jsonpackages/testing/src/playwright/__tests__/setupClerkTestingToken.test.tspackages/testing/tsconfig.test.jsonpackages/testing/vitest.config.mts
packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts (1)
18-19:⚠️ Potential issue | 🔴 CriticalTypecheck blocker:
fulfilled[].jsonisunknownbut dereferenced in assertionsLine 18 defines
jsonasunknown, then Lines 170/183/195 access nested properties directly. This will fail TS typecheck in strict mode.Suggested fix
+interface MockClerkClientPayload { + captcha_bypass?: boolean; +} + +interface MockClerkResponsePayload { + response?: MockClerkClientPayload; + client?: MockClerkClientPayload; +} + +interface MockFulfillOptions { + response?: unknown; + json?: MockClerkResponsePayload; +} + function createMockRoute(overrides: { url?: string; fetchStatus?: number; fetchJson?: unknown; fetchError?: Error } = {}) { @@ - const fulfilled: { response?: unknown; json?: unknown }[] = []; + const fulfilled: MockFulfillOptions[] = []; @@ - fulfill: vi.fn((opts: any) => { + fulfill: vi.fn((opts: MockFulfillOptions) => { fulfilled.push(opts); return Promise.resolve(); }),#!/bin/bash # Verify the type mismatch locations (read-only) rg -n "const fulfilled:|fulfilled\\[0\\]\\.json\\.(response|client)" packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts -n -C2Also applies to: 170-170, 183-183, 195-195
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts` around lines 18 - 19, The test declares fulfilled: { response?: unknown; json?: unknown }[] but later directly accesses nested properties on fulfilled[i].json (e.g., fulfilled[0].json.response), causing TS errors; change the json type to a concrete indexable shape (e.g., json?: Record<string, unknown> or a specific interface for the expected response structure) or add narrowing/casts before dereferencing (e.g., type guard or cast to any) so assertions compile; update the declaration of fulfilled in setupClerkTestingToken.test.ts (the fulfilled variable) and/or add local narrowing where fulfilled[].json is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts`:
- Around line 18-19: The test declares fulfilled: { response?: unknown; json?:
unknown }[] but later directly accesses nested properties on fulfilled[i].json
(e.g., fulfilled[0].json.response), causing TS errors; change the json type to a
concrete indexable shape (e.g., json?: Record<string, unknown> or a specific
interface for the expected response structure) or add narrowing/casts before
dereferencing (e.g., type guard or cast to any) so assertions compile; update
the declaration of fulfilled in setupClerkTestingToken.test.ts (the fulfilled
variable) and/or add local narrowing where fulfilled[].json is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: d460eca5-05f3-49be-b785-aca88a7bcaa6
📒 Files selected for processing (1)
packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts (1)
20-20:⚠️ Potential issue | 🔴 Critical
fulfilled[].jsonis typed asunknown, so these assertions are not type-safe and will fail typecheck.
fulfilled[0].json.response.../.client...dereferenceunknown. Givefulfilleda concrete payload type (or narrow before assertions) so this test compiles reliably.Proposed fix
+type FulfilledJson = + | { response?: { captcha_bypass?: boolean }; client?: { captcha_bypass?: boolean } } + | undefined; + +interface FulfilledEntry { + response?: unknown; + json?: FulfilledJson; +} + -const fulfilled: { response?: unknown; json?: unknown }[] = []; +const fulfilled: FulfilledEntry[] = [];#!/bin/bash # Verify this file typechecks under the package's Vitest typecheck setup pnpm -C packages/testing vitest typecheck src/playwright/__tests__/setupClerkTestingToken.test.tsAlso applies to: 172-172, 185-185, 197-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts` at line 20, The test declares fulfilled: { response?: unknown; json?: unknown }[] so later dereferences like fulfilled[0].json.response and .client are untyped and fail typecheck; update the type of fulfilled (or assert/narrow before use) to a concrete interface matching the expected payload (e.g., shape containing json.response and json.response.client) and use that type for the fulfilled array used in the assertions inside the setupClerkTestingToken tests (references: the fulfilled variable in setupClerkTestingToken.test.ts and any assertions that access .json.response or .json.response.client); alternatively, narrow fulfilled[0].json with a type guard or cast right before the assertions to satisfy the compiler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts`:
- Line 20: The test declares fulfilled: { response?: unknown; json?: unknown }[]
so later dereferences like fulfilled[0].json.response and .client are untyped
and fail typecheck; update the type of fulfilled (or assert/narrow before use)
to a concrete interface matching the expected payload (e.g., shape containing
json.response and json.response.client) and use that type for the fulfilled
array used in the assertions inside the setupClerkTestingToken tests
(references: the fulfilled variable in setupClerkTestingToken.test.ts and any
assertions that access .json.response or .json.response.client); alternatively,
narrow fulfilled[0].json with a type guard or cast right before the assertions
to satisfy the compiler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4bb1f252-ad1e-483d-abbc-7b9cf956b3ff
📒 Files selected for processing (1)
packages/testing/src/playwright/__tests__/setupClerkTestingToken.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes
setupClerkTestingToken()causingsignIn()timeouts when running Playwright with concurrent workers (--workers=2+).Root cause:
setupClerkTestingToken()is called fromsignIn(),goToAppHome(), andgoToRelative(). Each call registered a newbrowserContext.route()handler on the same context. With N stacked handlers, each FAPI request triggered N real HTTP requests viaroute.fetch(), amplifying rate-limit pressure. When FAPI returned 429/5xx, the catch block silently calledroute.continue(), swallowing the error. ClerkJS never received a valid response, never reachedloadedstate, andwaitForFunction()timed out.Changes:
WeakSet<BrowserContext>tracks contexts that already have a handler registered and returns early on subsequent calls. TheWeakSetis checked after parameter validation so an invalid call can't poison the set.ECONNREFUSED,ECONNRESET, etc.) with exponential backoff (500ms base, 2x multiplier) plus random jitter (0-250ms) to avoid thundering herd across workers.console.warnwith the request URL and status/error. For retryable HTTP statuses, fulfills with the raw error response so ClerkJS can handle it. For network errors, falls back toroute.continue().CLERK_TESTING_DEBUG=1to log retry attempts and duplicate setup skips.Test plan
pnpm buildpassesvitest runinpackages/testing)--workers=2+to confirm concurrent sign-in no longer times out