fix(clerk-js): Correct init retry logic#7128
Conversation
🦋 Changeset detectedLatest commit: 95c4f2f 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 |
jacekradko
force-pushed
the
fix/init-retry-logic
branch
from
f9b5aa2 to
aec61e8
Compare
@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: |
|
Found 4 test failures on Blacksmith runners:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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.
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.
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.
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
jacekradko
changed the title
|
Hello 👋 We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days. Thanks for being a part of the Clerk community! 🙏 |
- Add withRetry utility with exponential backoff and jitter - Refactor Clerk initialization to use withRetry wrapper - Preserve error cause in clerkErrorInitFailed for better debugging - Add comprehensive tests for retry utility
|
Hello 👋 We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days. Thanks for being a part of the Clerk community! 🙏 |
|
Hello 👋 We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days. Thanks for being a part of the Clerk community! 🙏 |
2 participants
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.