Skip to content

Conversation

@jacekradko
Copy link
Member

@jacekradko jacekradko commented Nov 3, 2025

Description

Refactor Clerk initialization retry logic to use a dedicated withRetry utility with exponential backoff

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • New Features

    • Added a configurable retry utility with jittered exponential backoff for startup operations.
  • Bug Fixes

    • Improved initialization resilience and recovery for transient/network failures, clearer error wrapping (init errors now include causes), and dev-browser auth recovery; UI can mount in degraded mode.
  • Tests

    • Added comprehensive tests for retry/backoff behavior and initialization retry scenarios.
  • Chores

    • Updated TypeScript libs and added a patch changeset.

✏️ Tip: You can customize this high-level summary in your review settings.

@changeset-bot
Copy link

changeset-bot bot commented Nov 3, 2025

🦋 Changeset detected

Latest commit: f2bf4a9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch

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

@vercel
Copy link

vercel bot commented Nov 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
clerk-js-sandbox Ready Ready Preview Comment Dec 1, 2025 3:23am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Introduces 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

Cohort / File(s) Summary
Retry utility implementation
packages/clerk-js/src/utils/retry.ts
Adds RetryOptions and withRetry<T>: exponential backoff, optional jitter (default true), sleep, and retry loop supporting sync/async shouldRetry.
Retry utility tests
packages/clerk-js/src/utils/__tests__/retry.test.ts
New comprehensive tests for withRetry: input validation, success/retry/exhaustion flows, jitter/backoff timing, and sync/async shouldRetry behaviors using fake timers.
Initialization refactor
packages/clerk-js/src/core/clerk.ts
Replaces ad-hoc two-attempt logic with withRetry; introduces initializeClerk, initEnvironmentPromise, initClient, and initComponents; adds degraded fallbacks (cached snapshot/local JWT), 4xx-aware handling, dev‑browser recovery flow, and token-polling coordination.
Initialization tests
packages/clerk-js/src/core/__tests__/clerk.test.ts
Adds tests covering init retry behavior: dev‑browser unauthenticated recovery, network-error propagation, retry attempts, and error cause propagation; updates test imports.
Error handling updates
packages/clerk-js/src/core/errors.ts
Changes clerkErrorInitFailed()clerkErrorInitFailed(error?: unknown) and throws an Error that includes the original error as its cause.
Build configuration
packages/clerk-js/tsconfig.json
Adds "es2022.error" to compilerOptions.lib.
Release metadata
.changeset/easy-bars-type.md
New changeset documenting the retry refactor and a patch release for @clerk/clerk-js.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Extra attention:
    • packages/clerk-js/src/core/clerk.ts — orchestration, shouldRetry semantics, degraded-path logic, and token-polling coordination.
    • packages/clerk-js/src/utils/retry.ts — backoff math, jitter behaviour, async shouldRetry, and timer correctness.
    • Tests — fake timers, mocks, retry assertions, and error cause propagation.

Poem

🐇 I jittered twice and hopped anew,
I chased lost envs through morning dew,
When client tripped or network sighed,
I tried again and stayed beside,
Now components wake — a carrot view! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring Clerk initialization retry logic to use a dedicated withRetry utility with exponential backoff, which is the primary focus across multiple files in the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/init-retry-logic

Comment @coderabbitai help to get the list of available commands and usage tips.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 3, 2025

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@7128

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7128

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7128

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7128

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7128

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7128

@clerk/elements

npm i https://pkg.pr.new/@clerk/elements@7128

@clerk/clerk-expo

npm i https://pkg.pr.new/@clerk/clerk-expo@7128

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7128

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7128

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7128

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7128

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7128

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7128

@clerk/clerk-react

npm i https://pkg.pr.new/@clerk/clerk-react@7128

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7128

@clerk/remix

npm i https://pkg.pr.new/@clerk/remix@7128

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7128

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7128

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7128

@clerk/themes

npm i https://pkg.pr.new/@clerk/themes@7128

@clerk/types

npm i https://pkg.pr.new/@clerk/types@7128

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7128

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7128

commit: f2bf4a9

@blacksmith-sh
Copy link

blacksmith-sh bot commented Nov 3, 2025

Found 4 test failures on Blacksmith runners:

Test View Logs
[chrome] › integration/tests/
components.test.ts:102:9 › component smoke tests @generic › long-running--next.appRoute
r.withEmailCodes › OrganizationProfile supports fallback
View Logs
[chrome] › integration/tests/
components.test.ts:102:9 › component smoke tests @generic › long-running--next.appRoute
r.withEmailCodes › UserButton supports fallback
View Logs
[chrome] › integration/tests/
resiliency.test.ts:87:7 › resiliency @generic › long-running--react.vite.withEmailCodes
› resiliency to not break devBrowser - dummy client and is not created on /</code><br /><code>client 4xx errors
View Logs
[chrome] › integration/tests/
session-tasks-multi-session.test.ts:33:9 › session tasks multi-session flow @nextjs › l
ong-running--next.appRouter.withSessionTasks › when switching sessions, navigate to tas
k
View Logs


Fix in Cursor

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between f47b5a3 and aec61e8.

📒 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.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/__tests__/retry.test.ts
  • packages/clerk-js/src/utils/retry.ts
  • packages/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.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/__tests__/retry.test.ts
  • packages/clerk-js/src/utils/retry.ts
  • packages/clerk-js/tsconfig.json
  • packages/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.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/__tests__/retry.test.ts
  • packages/clerk-js/src/utils/retry.ts
  • packages/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.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/__tests__/retry.test.ts
  • packages/clerk-js/src/utils/retry.ts
  • packages/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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for 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
Use const assertions for literal types: as const
Use satisfies operator 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 ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for 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.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/__tests__/retry.test.ts
  • packages/clerk-js/src/utils/retry.ts
  • packages/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.ts
  • packages/clerk-js/src/core/clerk.ts
  • packages/clerk-js/src/utils/__tests__/retry.test.ts
  • packages/clerk-js/src/utils/retry.ts
  • packages/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.ts
  • packages/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.ts
  • packages/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.ts
  • packages/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 good

Adding es2022.error keeps the compiler aware of Error options with cause, 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 on

Passing the underlying error as cause sharpens debugging and meshes with the retry utilities.

packages/clerk-js/src/utils/__tests__/retry.test.ts (1)

1-156: Great coverage on retry behavior

The 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 well

The 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 withRetry to treat the attempt as successful and skip the second retry, leaving this.environment uninitialized. 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0d7919b and 8e58d10.

📒 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/types instead of the deprecated @clerk/types alias

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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly 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 like Omit, Partial, and Pick for 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
Use const assertions with as const for literal types
Use satisfies operator 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/bash

Search 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/bash

Search 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/bash

Look 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/bash

Search for ClerkRuntimeError definition

rg -nP --type=ts 'class ClerkRuntimeError|type ClerkRuntimeError' -A 10 | head -50
</invoke_args>
</function_calls>

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 around cause instead of using any

The new load retry behavior suite nicely covers:

  • Single retry on dev-browser unauthenticated failures, including handleUnauthenticatedDevBrowser and mockClientFetch call counts.
  • Network error surfacing after retries, including wrapping via clerkErrorInitFailed and 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 any and better match the project’s type‑safety guidelines, you can narrow to ClerkRuntimeError explicitly:

-        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 any usage.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8e58d10 and 00c834d.

📒 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/types instead of the deprecated @clerk/types alias

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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly 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 like Omit, Partial, and Pick for 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
Use const assertions with as const for literal types
Use satisfies operator 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, and errorsModule are 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

RetryOptions is part of the public surface (and used by withRetry), but there’s no JSDoc describing semantics like default jitter, how shouldRetry is evaluated, or how many times maxAttempts is 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/centralized

The 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 nits

The core retry behavior (validation of maxAttempts, sync/async shouldRetry, last‑attempt handling, and default jitter behavior) is correct and easy to follow. A couple of small, non‑blocking nits you might consider:

  • throw lastError at Line 49 is now logically unreachable given the upfront maxAttempts guard and the fact every iteration either returns or throws; 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); since await already handles boolean | 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 00c834d and dbf02e9.

📒 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/types instead of the deprecated @clerk/types alias

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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly 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 like Omit, Partial, and Pick for 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
Use const assertions with as const for literal types
Use satisfies operator 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 withRetry from 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:

  1. Mixed return types: Success path returns implicit void (line 2750) while degraded path returns explicit null (line 2779)
  2. 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, withRetry won'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 clientResult from allSettled, completely ignoring the environment promise result. When requires_captcha triggers 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 if environment is an empty singleton object from a failed initialization. Consider checking this.environment?.id to 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.

📥 Commits

Reviewing files that changed from the base of the PR and between dbf02e9 and b15a32c.

📒 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/types instead of the deprecated @clerk/types alias

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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly 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 like Omit, Partial, and Pick for 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
Use const assertions with as const for literal types
Use satisfies operator 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 jacekradko changed the title fix(clerk-js): Correct init retry logic [WIP] fix(clerk-js): Correct init retry logic Nov 26, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between b55bf67 and 95e6d64.

📒 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.ts
  • 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/__tests__/clerk.test.ts
  • 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/__tests__/clerk.test.ts
  • 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/types instead of the deprecated @clerk/types alias

Files:

  • packages/clerk-js/src/core/__tests__/clerk.test.ts
  • 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/__tests__/clerk.test.ts
  • packages/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.ts
  • packages/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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly 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 like Omit, Partial, and Pick for 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
Use const assertions with as const for literal types
Use satisfies operator 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
  • 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). (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 allSettled with 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 clerkErrorInitFailed ensures failures are properly communicated to developers.

Comment on lines 2732 to 2747
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' });
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 possible

The test does a solid job verifying that, after retries are exhausted, load() fails with the generic init error, preserves the original ClerkRuntimeError as cause, calls clerkErrorInitFailed with the original error, and does not invoke handleUnauthenticatedDevBrowser.

The only small nit is that loadPromise is 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 cause via the rejection expectation alone, or by dropping the .rejects wrapper 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 95e6d64 and 2239274.

📒 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/types instead of the deprecated @clerk/types alias

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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly 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 like Omit, Partial, and Pick for 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
Use const assertions with as const for literal types
Use satisfies operator 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, and errorsModule are 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 timers

Storing/restoring Clerk.mountComponentRenderer in beforeEach/afterEach and resetting timers with vi.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 covered

This test accurately simulates a dev_browser_unauthenticated 401 on the first mount, verifies a single call to handleUnauthenticatedDevBrowser, 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 appropriately

Mocking SafeLocalStorage.getItem to return null, 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 on SafeLocalStorage at the module level and restoring it in finally keeps side effects contained. No changes requested.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 ClerkRuntimeError loses 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2239274 and f2bf4a9.

📒 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/types instead of the deprecated @clerk/types alias

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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly 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 like Omit, Partial, and Pick for 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
Use const assertions with as const for literal types
Use satisfies operator 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 withRetry utility is correctly added and used for the initialization retry logic.


2754-2788: LGTM!

The initClient function 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 initComponents function has proper guards to prevent double-mounting and ensures environment is available before mounting.


2796-2811: LGTM!

The allSettled pattern 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: The as any cast is necessary and cannot be removed.

The shouldRetry callback receives error: unknown from the retry mechanism, but isError() expects ClerkAPIResponseError. 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 unknown to ClerkAPIResponseError before calling isError()
  • Updating isError() to accept unknown and perform type validation internally

Likely an incorrect or invalid review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants