-
Notifications
You must be signed in to change notification settings - Fork 410
fix(clerk-js): Correct init retry logic #7128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: f2bf4a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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.
|
WalkthroughIntroduces a reusable withRetry utility and integrates it into Clerk initialization, splitting init into environment, client, and component phases with degraded fallbacks, 4xx-aware handling and dev‑browser recovery, updates error cause propagation, and adds tests for retry and init flows. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Clerk as Clerk.load()
participant Retry as withRetry
participant Init as initializeClerk
participant Env as Environment API
participant Client as Client API
participant Auth as AuthCookieService
App->>Clerk: load()
Clerk->>Retry: run initializeClerk (maxAttempts=2, jitter=true)
loop attempt
Retry->>Init: attempt init
Init->>Env: fetch environment (touch)
alt env success
Env-->>Init: env data
Init->>Client: fetch/create client
alt client success
Client-->>Init: client data
Init-->>Retry: success
else 4xx (dev-browser unauthenticated)
Client-->>Init: 4xx error
Init->>Auth: handleUnauthenticatedDevBrowser()
Auth-->>Init: recovery result
Init-->>Retry: success or non-retryable throw
else network/error
Client-->>Init: network error
Init-->>Retry: throw (retryable per policy)
end
else env fetch failed
Env-->>Init: error
Init->>Init: try cached snapshot / degraded path
Init->>Client: attempt with degraded env
Init-->>Retry: success or throw
end
alt shouldRetry & attempts left
Retry->>Retry: calculate backoff (with jitter)
Retry->>Retry: sleep(backoff)
else final attempt or no-retry
Retry-->>Clerk: throw error (wrapped with cause)
Clerk-->>App: initialization failed
end
end
Retry-->>Clerk: initialization succeeded
Clerk->>Clerk: initComponents, set ready state
Clerk-->>App: ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
f9b5aa2 to
aec61e8
Compare
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
|
Found 4 test failures on Blacksmith runners:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/clerk-js/src/core/__tests__/clerk.test.ts(3 hunks)packages/clerk-js/src/core/clerk.ts(2 hunks)packages/clerk-js/src/core/errors.ts(1 hunks)packages/clerk-js/src/utils/__tests__/retry.test.ts(1 hunks)packages/clerk-js/src/utils/retry.ts(1 hunks)packages/clerk-js/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/errors.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/utils/retry.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/errors.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/utils/retry.tspackages/clerk-js/tsconfig.jsonpackages/clerk-js/src/core/__tests__/clerk.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/errors.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/utils/retry.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/core/errors.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/utils/retry.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/core/errors.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/utils/retry.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/core/errors.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/utils/retry.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/utils/__tests__/retry.test.tspackages/clerk-js/src/core/__tests__/clerk.test.ts
packages/*/tsconfig.json
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Type checking must be performed with TypeScript and publint.
Files:
packages/clerk-js/tsconfig.json
🧬 Code graph analysis (3)
packages/clerk-js/src/core/clerk.ts (7)
packages/clerk-js/src/core/resources/Environment.ts (1)
Environment(17-100)packages/clerk-js/src/utils/localStorage.ts (1)
SafeLocalStorage(17-68)packages/shared/src/errors/helpers.ts (3)
is4xxError(33-36)isNetworkError(43-47)isClerkRuntimeError(83-85)packages/clerk-js/src/core/jwt-client.ts (1)
createClientFromJwt(19-100)packages/clerk-js/src/utils/errors.ts (1)
isError(4-6)packages/clerk-js/src/utils/retry.ts (1)
withRetry(19-46)packages/clerk-js/src/core/errors.ts (1)
clerkErrorInitFailed(19-21)
packages/clerk-js/src/utils/__tests__/retry.test.ts (1)
packages/clerk-js/src/utils/retry.ts (1)
withRetry(19-46)
packages/clerk-js/src/core/__tests__/clerk.test.ts (2)
packages/clerk-js/src/core/clerk.ts (1)
Clerk(198-2989)packages/clerk-js/src/core/auth/AuthCookieService.ts (1)
AuthCookieService(40-255)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/clerk-js/tsconfig.json (1)
10-10: Cause-enabled lib entry looks goodAdding
es2022.errorkeeps the compiler aware ofErroroptions withcause, aligning tsconfig with the new error handling.packages/clerk-js/src/core/errors.ts (1)
19-21: Wrapping the init failure with a cause is spot onPassing the underlying error as
causesharpens debugging and meshes with the retry utilities.packages/clerk-js/src/utils/__tests__/retry.test.ts (1)
1-156: Great coverage on retry behaviorThe suite nails first-try success, retry gating, async predicates, and backoff timing—including jitter—so regressions here should be caught quickly.
packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
1-282: Retry regression tests read wellThe new fixtures and expectations around dev-browser recovery and network error propagation exercise the retry path end-to-end, which should keep future init changes safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/clerk-js/src/core/clerk.ts (1)
2732-2745: Rethrow environment fetch errors when no cache exists.When the environment fetch fails and no cached snapshot is available in localStorage, this catch block resolves successfully instead of rethrowing. This causes
withRetryto treat the attempt as successful and skip the second retry, leavingthis.environmentuninitialized. Users will see a broken UI until they hard refresh.This was flagged in a previous review. Please apply the suggested fix:
- .catch(() => { + .catch(error => { ++initializationDegradedCounter; const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( CLERK_ENVIRONMENT_STORAGE_ENTRY, null, ); if (environmentSnapshot) { this.updateEnvironment(new Environment(environmentSnapshot)); + return; } + + throw error; });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/clerk-js/src/core/__tests__/clerk.test.ts(3 hunks)packages/clerk-js/src/core/clerk.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/core/tests/clerk.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/clerk.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/clerk.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/clerk.ts (6)
packages/clerk-js/src/core/resources/Environment.ts (1)
Environment(18-104)packages/shared/src/errors/helpers.ts (3)
is4xxError(33-36)isNetworkError(43-47)isClerkRuntimeError(83-85)packages/clerk-js/src/core/jwt-client.ts (1)
createClientFromJwt(19-100)packages/shared/src/utils/allSettled.ts (1)
allSettled(5-15)packages/clerk-js/src/utils/retry.ts (1)
withRetry(19-46)packages/clerk-js/src/core/errors.ts (1)
clerkErrorInitFailed(19-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (33)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Static analysis
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/core/clerk.ts (1)
2815-2830: I'll help you verify the network error detection and type safety concerns. Let me investigate the codebase to validate the review comment.
<function_calls>
<invoke_name>shell
<invoke_args>
#!/bin/bashSearch for isNetworkError usage and how it's implemented
rg -nP --type=ts -C3 'isNetworkError' | head -60
</invoke_args>
</invoke_function_calls>
<invoke_name>shell
<invoke_args>
#!/bin/bashSearch for ClerkRuntimeError definition and network_error code
rg -nP --type=ts 'network_error' | head -40
</invoke_args>
</function_calls>
<invoke_name>shell
<invoke_args>
#!/bin/bashLook at the clerk.ts file around the lines in question
cat -n packages/clerk-js/src/core/clerk.ts | sed -n '2800,2850p'
</invoke_args>
</function_calls>
<invoke_name>shell
<invoke_args>
#!/bin/bashSearch for ClerkRuntimeError definition
rg -nP --type=ts 'class ClerkRuntimeError|type ClerkRuntimeError' -A 10 | head -50
</invoke_args>
</function_calls>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
162-283: Retry behavior tests are robust; consider tightening typing aroundcauseinstead of usinganyThe new
load retry behaviorsuite nicely covers:
- Single retry on dev-browser unauthenticated failures, including
handleUnauthenticatedDevBrowserandmockClientFetchcall counts.- Network error surfacing after retries, including wrapping via
clerkErrorInitFailedand verifying that dev-browser recovery is not invoked.One small improvement: in the network error test you currently type‑erode the error cause:
const err = await loadPromise.catch(e => e); expect(err).toBeInstanceOf(Error); expect((err as Error).message).toMatch(/Something went wrong initializing Clerk/); const cause = (err as Error).cause as any; expect(cause).toBeDefined(); expect(cause.code).toBe('network_error'); expect(cause.clerkRuntimeError).toBe(true);To avoid
anyand better match the project’s type‑safety guidelines, you can narrow toClerkRuntimeErrorexplicitly:- const err = await loadPromise.catch(e => e); - expect(err).toBeInstanceOf(Error); - expect((err as Error).message).toMatch(/Something went wrong initializing Clerk/); - const cause = (err as Error).cause as any; - expect(cause).toBeDefined(); - expect(cause.code).toBe('network_error'); - expect(cause.clerkRuntimeError).toBe(true); + const err = await loadPromise.catch(e => e); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/Something went wrong initializing Clerk/); + const cause = (err as { cause?: ClerkRuntimeError }).cause; + expect(cause).toBeInstanceOf(ClerkRuntimeError); + expect(cause?.code).toBe('network_error'); + expect(cause?.clerkRuntimeError).toBe(true);This keeps the same assertions while eliminating the new
anyusage.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/clerk-js/src/core/__tests__/clerk.test.ts(3 hunks)packages/clerk-js/src/utils/__tests__/retry.test.ts(1 hunks)packages/clerk-js/src/utils/retry.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/clerk-js/src/utils/retry.ts
- packages/clerk-js/src/utils/tests/retry.test.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
1-1: New imports for runtime errors, auth cookies, and error helpers are correctly integrated
ClerkRuntimeError,AuthCookieService, anderrorsModuleare all exercised in the new retry tests below; there are no unused imports or obvious wiring issues here.Also applies to: 16-16, 19-19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/clerk-js/src/utils/retry.ts (3)
1-5: Add JSDoc for the public retry API
RetryOptionsis part of the public surface (and used bywithRetry), but there’s no JSDoc describing semantics like default jitter, howshouldRetryis evaluated, or how many timesmaxAttemptsis tried. Per guidelines, public APIs should be documented with@param,@returns, and@throws(and ideally an@example) so usage is clear.
7-17: Consider making backoff configuration more explicit/centralizedThe exponential backoff formula (
2 ** (attempt + 1) * 1000) and jitter factor (0.5 + Math.random() * 0.5) are hard‑coded here. If other call sites or future retries need different policies (e.g., max cap, different base delay), extracting these constants (or a small config object) would make tuning easier and avoid scattering magic numbers.
19-49: Retry loop logic looks solid; only minor nitsThe core retry behavior (validation of
maxAttempts, sync/asyncshouldRetry, last‑attempt handling, and defaultjitterbehavior) is correct and easy to follow. A couple of small, non‑blocking nits you might consider:
throw lastErrorat Line 49 is now logically unreachable given the upfrontmaxAttemptsguard and the fact every iteration eitherreturns orthrows; it can be removed to reduce noise if you don’t rely on it for type‑checking clarity.- You could simplify Line 32 to
const shouldRetry = await options.shouldRetry(error);sinceawaitalready handlesboolean | Promise<boolean>cleanly.Functionally this is fine as is; these are just cleanups.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/clerk-js/src/utils/__tests__/retry.test.ts(1 hunks)packages/clerk-js/src/utils/retry.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/utils/tests/retry.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/utils/retry.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/utils/retry.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/utils/retry.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/utils/retry.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/utils/retry.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/utils/retry.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/utils/retry.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/clerk-js/src/core/clerk.ts (3)
2732-2745: [Duplicate] Environment fetch failure still swallowed when no cache exists.This issue was previously flagged and remains unaddressed. When the environment fetch fails without a cached snapshot, the error is silently caught at line 2735, preventing
withRetryfrom attempting a second pass. The environment remains uninitialized (empty singleton), which will cause downstream issues.Apply the fix suggested in the previous review to rethrow when no snapshot is available:
- .catch(() => { + .catch(error => { ++initializationDegradedCounter; const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( CLERK_ENVIRONMENT_STORAGE_ENTRY, null, ); if (environmentSnapshot) { this.updateEnvironment(new Environment(environmentSnapshot)); + return; } + + throw error; });
2747-2781: [Duplicate] initClient has inconsistent return type and missing JWT validation.Both issues flagged in previous review remain unresolved:
- Mixed return types: Success path returns implicit
void(line 2750) while degraded path returns explicitnull(line 2779)- Unvalidated JWT fallback: When client fetch fails with non-4xx error, the code creates a client from JWT (line 2762) without validating whether it's valid, then updates state. If the JWT cookie is missing/invalid,
withRetrywon't trigger because the function completes successfully.Apply the fix from the previous review:
+const initClient = async (): Promise<void> => { -const initClient = async () => { return Client.getOrCreateInstance() .fetch() .then(res => this.updateClient(res)) .catch(async e => { if (is4xxError(e)) { throw e; } ++initializationDegradedCounter; const jwtInCookie = this.#authService?.getSessionCookie(); const localClient = createClientFromJwt(jwtInCookie); + if (!localClient || localClient.id === 'client_init') { + throw e; + } + this.updateClient(localClient); this.#authService?.stopPollingForToken(); await this.session ?.getToken({ skipCache: true }) .catch(() => null) .finally(() => { this.#authService?.startPollingForToken(); }); - - return null; }); };
2789-2800: [Duplicate] Environment initialization result still ignored before CAPTCHA handling.This issue from the previous review remains unaddressed. Line 2789 destructures only
clientResultfromallSettled, completely ignoring the environment promise result. Whenrequires_captchatriggers component mounting (line 2795), an uninitialized environment will cause rendering errors.Apply the fix from the previous review:
const [, clientResult] = await allSettled([initEnvironmentPromise, initClient()]); + +// Ensure environment is initialized before handling CAPTCHA or mounting components +if (!this.environment?.id) { + throw new Error('Environment initialization failed'); +} if (clientResult.status === 'rejected') { const e = clientResult.reason; if (isError(e, 'requires_captcha')) { initComponents(); await initClient(); } else { throw e; } }
🧹 Nitpick comments (1)
packages/clerk-js/src/core/clerk.ts (1)
2783-2787: Consider stricter environment validation before mounting components.Line 2784 uses a truthy check (
this.environment) before mounting components. This passes even ifenvironmentis an empty singleton object from a failed initialization. Consider checkingthis.environment?.idto confirm the environment is fully initialized.-if (Clerk.mountComponentRenderer && !this.#componentControls && this.environment) { +if (Clerk.mountComponentRenderer && !this.#componentControls && this.environment?.id) { this.#componentControls = Clerk.mountComponentRenderer(this, this.environment, this.#options); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/core/clerk.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/clerk.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/clerk.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (33)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/clerk-js/src/core/__tests__/clerk.test.ts(3 hunks)packages/clerk-js/src/core/clerk.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
packages/clerk-js/src/core/__tests__/clerk.test.ts (3)
197-235: LGTM: Dev browser recovery test is well-structured.The test correctly simulates the dev browser unauthenticated scenario, verifies the recovery flow (handleUnauthenticatedDevBrowser), and asserts the retry behavior with proper use of fake timers.
237-282: LGTM: Network error handling test is comprehensive.The test properly validates that network errors trigger retries, are surfaced via clerkErrorInitFailed with the original error as cause, and don't incorrectly trigger dev browser authentication recovery. The error cause validation (lines 263-269) is particularly thorough.
284-319: LGTM: Environment fetch retry test is well-designed.The test correctly validates that environment fetch failures without a cached snapshot trigger a full retry of the initialization flow, including both environment and client fetches.
packages/clerk-js/src/core/clerk.ts (2)
2791-2815: LGTM: Initialization flow properly validates environment before mounting components.The use of
allSettledwith environment validation (lines 2793-2795) before proceeding ensures components are never mounted with an uninitialized environment. The CAPTCHA handling correctly mounts components after environment is confirmed valid, then retries client initialization.
2817-2840: LGTM: Retry wrapper is well-implemented with appropriate conditions.The retry logic properly handles offline detection, dev browser authentication recovery, and network errors. The max 2 attempts with jitter is a reasonable balance between resilience and avoiding excessive retries. Error surfacing via
clerkErrorInitFailedensures failures are properly communicated to developers.
| const initEnvironmentPromise = Environment.getInstance() | ||
| .fetch({ touch: shouldTouchEnv }) | ||
| .then(res => this.updateEnvironment(res)) | ||
| .catch(() => { | ||
| ++initializationDegradedCounter; | ||
| const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( | ||
| CLERK_ENVIRONMENT_STORAGE_ENTRY, | ||
| null, | ||
| ); | ||
|
|
||
| try { | ||
| const initEnvironmentPromise = Environment.getInstance() | ||
| .fetch({ touch: shouldTouchEnv }) | ||
| .then(res => this.updateEnvironment(res)) | ||
| .catch(() => { | ||
| ++initializationDegradedCounter; | ||
| const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( | ||
| CLERK_ENVIRONMENT_STORAGE_ENTRY, | ||
| null, | ||
| ); | ||
| if (environmentSnapshot) { | ||
| this.updateEnvironment(new Environment(environmentSnapshot)); | ||
| } else { | ||
| throw new ClerkRuntimeError('Failed to fetch environment', { code: 'network_error' }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve original error when throwing after failed environment fetch.
When the environment fetch fails and no cache exists, line 2745 creates a new ClerkRuntimeError without preserving the original error as the cause. This loses valuable debugging context about what actually failed.
Apply this diff to preserve the original error:
- .catch(() => {
+ .catch(error => {
++initializationDegradedCounter;
const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>(
CLERK_ENVIRONMENT_STORAGE_ENTRY,
null,
);
if (environmentSnapshot) {
this.updateEnvironment(new Environment(environmentSnapshot));
} else {
- throw new ClerkRuntimeError('Failed to fetch environment', { code: 'network_error' });
+ throw new ClerkRuntimeError('Failed to fetch environment', {
+ code: 'network_error',
+ cause: error,
+ });
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const initEnvironmentPromise = Environment.getInstance() | |
| .fetch({ touch: shouldTouchEnv }) | |
| .then(res => this.updateEnvironment(res)) | |
| .catch(() => { | |
| ++initializationDegradedCounter; | |
| const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( | |
| CLERK_ENVIRONMENT_STORAGE_ENTRY, | |
| null, | |
| ); | |
| try { | |
| const initEnvironmentPromise = Environment.getInstance() | |
| .fetch({ touch: shouldTouchEnv }) | |
| .then(res => this.updateEnvironment(res)) | |
| .catch(() => { | |
| ++initializationDegradedCounter; | |
| const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( | |
| CLERK_ENVIRONMENT_STORAGE_ENTRY, | |
| null, | |
| ); | |
| if (environmentSnapshot) { | |
| this.updateEnvironment(new Environment(environmentSnapshot)); | |
| } else { | |
| throw new ClerkRuntimeError('Failed to fetch environment', { code: 'network_error' }); | |
| } | |
| }); | |
| const initEnvironmentPromise = Environment.getInstance() | |
| .fetch({ touch: shouldTouchEnv }) | |
| .then(res => this.updateEnvironment(res)) | |
| .catch(error => { | |
| +initializationDegradedCounter; | |
| const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( | |
| CLERK_ENVIRONMENT_STORAGE_ENTRY, | |
| null, | |
| ); | |
| if (environmentSnapshot) { | |
| this.updateEnvironment(new Environment(environmentSnapshot)); | |
| } else { | |
| throw new ClerkRuntimeError('Failed to fetch environment', { | |
| code: 'network_error', | |
| cause: error, | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 2732 to 2747, the catch
handler for the Environment.getInstance().fetch call throws a new
ClerkRuntimeError without preserving the original error; capture the caught
error (e.g. catch(err) { ... }) and when throwing the ClerkRuntimeError include
the original error as the cause (or wrap/attach it in the error payload) so the
original stack/message is preserved for debugging while keeping the existing
degraded-counter increment and fallback-to-cache logic intact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
237-282: Network‑error propagation test is thorough; minor simplification possibleThe test does a solid job verifying that, after retries are exhausted,
load()fails with the generic init error, preserves the originalClerkRuntimeErrorascause, callsclerkErrorInitFailedwith the original error, and does not invokehandleUnauthenticatedDevBrowser.The only small nit is that
loadPromiseis both:
- passed to
expect(...).rejects, and- separately awaited via
.catch(e => e).That’s correct but slightly redundant. You could simplify by asserting both the message and the
causevia the rejection expectation alone, or by dropping the.rejectswrapper and asserting directly on the caught error, whichever you find clearer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/core/__tests__/clerk.test.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/__tests__/clerk.test.ts (4)
packages/shared/src/types/clerk.ts (1)
Clerk(170-957)packages/clerk-js/src/core/clerk.ts (1)
Clerk(205-3152)packages/clerk-js/src/core/auth/AuthCookieService.ts (1)
AuthCookieService(43-262)packages/shared/src/error.ts (1)
ClerkRuntimeError(13-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (34)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: Static analysis
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/clerk-js/src/core/__tests__/clerk.test.ts (4)
1-1: New imports are coherent and fully utilized
ClerkRuntimeError,localStorageModule,AuthCookieService, anderrorsModuleare all used in the new tests, match existing module boundaries, and don’t introduce unused symbols or layering issues. No changes requested.Also applies to: 16-17, 20-20
163-195: Setup for retry tests correctly isolates static overrides and timersStoring/restoring
Clerk.mountComponentRendererinbeforeEach/afterEachand resetting timers withvi.useRealTimers()prevents cross-test pollution and makes these tests safe even if other suites also touch timers or the static renderer. The local factory helpers keep the mocks focused and readable. No changes requested.
197-235: Dev‑browser unauthenticated retry scenario is well coveredThis test accurately simulates a
dev_browser_unauthenticated401 on the first mount, verifies a single call tohandleUnauthenticatedDevBrowser, and asserts a second mount and client fetch. That gives good confidence that the new retry logic handles lost dev-browser auth once and then recovers cleanly. No changes requested.
284-314: Environment‑fetch failure retry behavior is validated appropriatelyMocking
SafeLocalStorage.getItemto returnnull, forcing the first environment fetch to reject and the second to succeed, and then asserting two environment and client fetches gives good coverage of the “no cached snapshot, retry on env failure” path. Spying onSafeLocalStorageat the module level and restoring it infinallykeeps side effects contained. No changes requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/clerk-js/src/core/clerk.ts (1)
2735-2751: Preserve original error when throwing after failed environment fetch.When the environment fetch fails and no cache exists, the new
ClerkRuntimeErrorloses the original error context. This makes debugging harder.Apply this diff to preserve the original error:
- .catch(err => { + .catch((err: unknown) => { // Let 4xx errors (like dev_browser_unauthenticated) bubble up to be handled by retry logic if (is4xxError(err)) { throw err; } ++initializationDegradedCounter; const environmentSnapshot = SafeLocalStorage.getItem<EnvironmentJSONSnapshot | null>( CLERK_ENVIRONMENT_STORAGE_ENTRY, null, ); if (environmentSnapshot) { this.updateEnvironment(new Environment(environmentSnapshot)); } else { - throw new ClerkRuntimeError('Failed to fetch environment', { code: 'network_error' }); + throw new ClerkRuntimeError('Failed to fetch environment', { + code: 'network_error', + cause: err, + }); } });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/core/clerk.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/clerk.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/clerk.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/clerk.ts (5)
packages/clerk-js/src/core/resources/Environment.ts (1)
Environment(18-104)packages/clerk-js/src/core/resources/Client.ts (1)
Client(16-164)packages/clerk-js/src/core/jwt-client.ts (1)
createClientFromJwt(19-100)packages/clerk-js/src/utils/retry.ts (1)
withRetry(19-50)packages/clerk-js/src/core/errors.ts (1)
clerkErrorInitFailed(19-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (6)
packages/clerk-js/src/core/clerk.ts (6)
144-144: LGTM!The import for
withRetryutility is correctly added and used for the initialization retry logic.
2754-2788: LGTM!The
initClientfunction correctly implements:
- 4xx error re-throwing for retry handling
- Graceful degradation with JWT fallback for non-4xx errors
- Proper token polling lifecycle management around the recovery attempt
2790-2794: LGTM!The
initComponentsfunction has proper guards to prevent double-mounting and ensures environment is available before mounting.
2796-2811: LGTM!The
allSettledpattern correctly handles concurrent initialization with proper error propagation. Environment failures are checked before proceeding with CAPTCHA handling, addressing the previous review concern.
2813-2820: LGTM!Post-initialization flow is well-structured with proper early return on FAPI redirect and deferred component mounting.
2822-2844: Theas anycast is necessary and cannot be removed.The
shouldRetrycallback receiveserror: unknownfrom the retry mechanism, butisError()expectsClerkAPIResponseError. The type assertion is required to pass the unknown error to the function. Removing it would cause a TypeScript compilation error.A better approach would be to improve the type safety by either:
- Adding a type guard that narrows
unknowntoClerkAPIResponseErrorbefore callingisError()- Updating
isError()to acceptunknownand perform type validation internallyLikely an incorrect or invalid review comment.
Description
Refactor Clerk initialization retry logic to use a dedicated withRetry utility with exponential backoff
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.