feat(shared): Introduce base ClerkError and type guard helpers #6985
Conversation
🦋 Changeset detectedLatest commit: 8662db6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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.
|
WalkthroughRefactors error handling into a centralized, typed hierarchy: adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Svc as Remote Service
participant Lib as Library
participant Parser as parseErrors
participant EApi as ClerkAPIError
participant EResp as ClerkAPIResponseError
participant App as Consumer
Note over Svc,Lib: HTTP error response with JSON { errors: [...] }
Svc-->>Lib: HTTP error JSON
Lib->>Parser: parseErrors(json.errors)
Parser-->>Lib: ClerkAPIErrorJSON[]
loop map errors
Lib->>EApi: new ClerkAPIError(jsonItem)
end
Lib->>EResp: new ClerkAPIResponseError(message, { status, clerkTraceId, retryAfter, errors })
App->>EResp: isClerkApiResponseError(err)?
alt true
App->>EResp: read status, retryAfter, clerkTraceId, errors
else
App->>App: fallback -> isClerkError / isClerkRuntimeError
end
sequenceDiagram
autonumber
participant Lib as Library
participant ERt as ClerkRuntimeError
participant Guard as isClerkRuntimeError
participant App as Consumer
Lib->>ERt: new ClerkRuntimeError(message, { code, cause, docsUrl })
App->>Guard: isClerkRuntimeError(err)?
alt true
App->>ERt: inspect code, longMessage, docsUrl
else
App->>App: handle other ClerkError types via isClerkError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/errors/helpers.ts (1)
63-65: Critical: Type guard incorrectly identifies ClerkAPIResponseError.The check
'clerkError' in erron line 64 will returntruefor ANY Clerk error type becauseclerkErroris a property of the baseClerkErrorclass. This means it will incorrectly identifyClerkRuntimeErrorand other Clerk errors asClerkAPIResponseError.You should either:
- Use the
isClerkApiResponseErrortype guard exported fromclerkApiResponseError.ts- Check for a property specific to ClerkAPIResponseError (e.g.,
'status' in err)- Use
instanceof ClerkAPIResponseError+import { isClerkApiResponseError as isClerkApiResponseErrorGuard } from './clerkApiResponseError'; + /** * Checks if the provided error is a ClerkAPIResponseError. * * @internal */ export function isClerkAPIResponseError(err: any): err is ClerkAPIResponseError { - return err && 'clerkError' in err; + return isClerkApiResponseErrorGuard(err); }
🧹 Nitpick comments (6)
packages/shared/src/errors/clerkError.ts (1)
43-49: Review message formatting for readability.The current formatting adds multiple newlines (
\n\n) which may create excessive whitespace in error messages. Consider simplifying:const formatMessage = (msg: string, code: string, docsUrl: string | undefined) => { - msg = `${this.name}: ${msg.trim()}\n\n(code="${code}")\n\n`; + msg = `${this.name}: ${msg.trim()}\n(code="${code}")`; if (__DEV__) { - msg += `\n\nDocs: ${docsUrl}`; + msg += `\nDocs: ${docsUrl}`; } return msg; };packages/shared/src/errors/clerkApiError.ts (1)
10-10: Avoidanyas default generic type.The default generic type
anydefeats type safety. Consider usingRecord<string, unknown>or requiring the generic to be specified.-export class ClerkAPIError<Meta extends ClerkApiErrorMeta = any> implements ClerkAPIErrorInterface { +export class ClerkAPIError<Meta extends ClerkApiErrorMeta = Record<string, unknown>> extends Error implements ClerkAPIErrorInterface {packages/shared/src/errors/createErrorTypeGuard.ts (1)
19-36: Review: Consider if dual-call signature is necessary.The implementation is technically correct, but the dual-call signature pattern (standalone function + method style) adds complexity. In practice, most error handling code uses standalone type guards like
if (isMyError(error))rather than method calls likeif (error.isMyError()).Unless there's a specific requirement for the method-style usage, consider simplifying to a standard standalone type guard:
export function createErrorTypeGuard<T extends new (...args: any[]) => Error>( ErrorClass: T, ): (error: unknown) => error is InstanceType<T> { return (error: unknown): error is InstanceType<T> => { return error instanceof ErrorClass; }; }However, if the method-style usage is intentional for API design, the current implementation is correct.
packages/shared/src/errors/clerkApiResponseError.ts (2)
22-28: Review constructor parameter passing to base class.The constructor passes a hardcoded
code: 'api_response_error'to the base ClerkError. Consider:
- Whether this code should be configurable
- Whether
longMessage,docsUrl, andcausefromoptionsshould be passed to the base class for consistencyconstructor(message: string, options: ClerkAPIResponseOptions) { const { data: errorsJson, status, clerkTraceId, retryAfter } = options; - super({ ...options, message, code: 'api_response_error' }); + super({ message, code: 'api_response_error', ...options }); this.status = status; this.clerkTraceId = clerkTraceId; this.retryAfter = retryAfter; this.errors = errorsJson.map(e => new ClerkAPIError(e)); }
31-41: Consider improving error serialization in toString().Using
JSON.stringify(e)on line 33 to serialize errors may not be optimal because:
- ClerkAPIError instances may not serialize well (circular references, methods)
- The output may be difficult to read in logs
Consider using a more structured format or calling toString() on each error:
public toString() { - let message = `[${this.name}]\nMessage:${this.message}\nStatus:${this.status}\nSerialized errors: ${this.errors.map( - e => JSON.stringify(e), - )}`; + let message = `[${this.name}]\nMessage:${this.message}\nStatus:${this.status}\nErrors:\n${this.errors.map( + e => ` - ${e.code}: ${e.message}`, + ).join('\n')}`; if (this.clerkTraceId) { message += `\nClerk Trace ID: ${this.clerkTraceId}`; } return message; }packages/shared/src/errors/helpers.ts (1)
83-85: Update type guard to use new implementation.The check for
'clerkRuntimeError'uses the deprecated flag. Consider using the newisClerkRuntimeErrortype guard exported fromclerkRuntimeError.tsfor consistency with the refactored error system.+import { isClerkRuntimeError as isClerkRuntimeErrorGuard } from './clerkRuntimeError'; + /** * Checks if the provided error object is an instance of ClerkRuntimeError. * * @param err - The error object to check. * @returns True if the error is a ClerkRuntimeError, false otherwise. * * @example * const error = new ClerkRuntimeError('An error occurred'); * if (isClerkRuntimeError(error)) { * // Handle ClerkRuntimeError * console.error('ClerkRuntimeError:', error.message); * } else { * // Handle other errors * console.error('Other error:', error.message); * } */ export function isClerkRuntimeError(err: any): err is ClerkRuntimeError { - return 'clerkRuntimeError' in err; + return isClerkRuntimeErrorGuard(err); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
.changeset/refactor-error-handling-system.md(1 hunks)packages/shared/src/errors/apiResponseError.ts(0 hunks)packages/shared/src/errors/clerkApiError.ts(1 hunks)packages/shared/src/errors/clerkApiResponseError.ts(1 hunks)packages/shared/src/errors/clerkError.ts(1 hunks)packages/shared/src/errors/clerkRuntimeError.ts(1 hunks)packages/shared/src/errors/createErrorTypeGuard.ts(1 hunks)packages/shared/src/errors/helpers.ts(1 hunks)packages/shared/src/errors/runtimeError.ts(0 hunks)packages/shared/src/errors/webAuthNError.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/shared/src/errors/runtimeError.ts
- packages/shared/src/errors/apiResponseError.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/shared/src/errors/createErrorTypeGuard.tspackages/shared/src/errors/clerkError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/helpers.tspackages/shared/src/errors/webAuthNError.tspackages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiError.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/shared/src/errors/createErrorTypeGuard.tspackages/shared/src/errors/clerkError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/helpers.tspackages/shared/src/errors/webAuthNError.tspackages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiError.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/errors/createErrorTypeGuard.tspackages/shared/src/errors/clerkError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/helpers.tspackages/shared/src/errors/webAuthNError.tspackages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiError.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/errors/createErrorTypeGuard.tspackages/shared/src/errors/clerkError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/helpers.tspackages/shared/src/errors/webAuthNError.tspackages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiError.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/shared/src/errors/createErrorTypeGuard.tspackages/shared/src/errors/clerkError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/helpers.tspackages/shared/src/errors/webAuthNError.tspackages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiError.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/shared/src/errors/createErrorTypeGuard.tspackages/shared/src/errors/clerkError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/helpers.tspackages/shared/src/errors/webAuthNError.tspackages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiError.ts
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/refactor-error-handling-system.md
🧬 Code graph analysis (3)
packages/shared/src/errors/clerkApiResponseError.ts (4)
packages/shared/src/errors/clerkError.ts (2)
ClerkErrorParams(1-26)ClerkError(34-61)packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-24)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(19-37)
packages/shared/src/errors/clerkRuntimeError.ts (3)
packages/shared/src/errors/clerkError.ts (2)
ClerkErrorParams(1-26)ClerkError(34-61)packages/shared/src/errors/helpers.ts (1)
isClerkRuntimeError(83-85)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(19-37)
packages/shared/src/errors/clerkApiError.ts (1)
packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (8)
packages/shared/src/errors/clerkError.ts (2)
66-68: LGTM! Well-implemented type guard.The type guard correctly checks for the
clerkErrorflag and validates both the object type and the flag value.
36-36: Makenameproperty readonly for consistency.The
nameproperty should bereadonlyto match the other properties in the class and prevent accidental modification.- readonly name: string = 'ClerkError'; + readonly name = 'ClerkError';Likely an incorrect or invalid review comment.
packages/shared/src/errors/clerkApiResponseError.ts (1)
55-55: LGTM! Type guard correctly uses the factory.The type guard is properly created using
createErrorTypeGuardand will work with instanceof checks.packages/shared/src/errors/webAuthNError.ts (1)
1-1: LGTM! Import path correctly updated.The import path has been updated to reflect the renamed module structure from
runtimeError.tstoclerkRuntimeError.ts..changeset/refactor-error-handling-system.md (1)
1-13: LGTM! Comprehensive changeset documentation.The changeset clearly documents all the major changes in the error handling refactor, including the new base class, renamed files, new type guards, and deprecation notices.
packages/shared/src/errors/clerkRuntimeError.ts (3)
16-19: Acknowledge backward compatibility approach for deprecated flag.Setting the
clerkRuntimeErrorflag while marking it as deprecated is a good approach for backward compatibility during the migration period. This allows existing code checking for this flag to continue working while new code can use the baseclerkErrorflag.
38-38: LGTM! Type guard correctly uses the factory.The type guard is properly created using
createErrorTypeGuardand provides both standalone and method-style usage patterns.
20-20: Makenameproperty readonly for consistency.The
nameproperty should bereadonlyto match the readonly properties in the base ClerkError class and prevent modification.- readonly name = 'ClerkRuntimeError'; + readonly name = 'ClerkRuntimeError' as const;Likely an incorrect or invalid review comment.
| } | ||
|
|
||
| export class ClerkAPIResponseError extends ClerkError implements ClerkAPIResponseErrorInterface { | ||
| name = 'ClerkAPIResponseError'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make name property readonly for consistency.
The name property should be readonly to prevent modification and align with the readonly properties in the base ClerkError class.
- name = 'ClerkAPIResponseError';
+ readonly name = 'ClerkAPIResponseError';📝 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.
| name = 'ClerkAPIResponseError'; | |
| readonly name = 'ClerkAPIResponseError'; |
🤖 Prompt for AI Agents
In packages/shared/src/errors/clerkApiResponseError.ts around line 16, the class
sets name = 'ClerkAPIResponseError' but it's not marked readonly; change the
declaration to make the name property readonly (e.g., readonly name =
'ClerkAPIResponseError') so it cannot be modified and matches the readonly
pattern used in the base ClerkError class.
nikosdouvlis
force-pushed
the
nikos/userland-errors-p-6
branch
from
bdc1146 to
8ed495e
Compare
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/shared/src/errors/clerkApiResponseError.ts (1)
16-16: Static property should be readonly.The static
nameproperty should be markedreadonlyto prevent modification and maintain consistency.
🧹 Nitpick comments (1)
packages/shared/src/errors/clerkApiResponseError.ts (1)
33-35: Consider more concise error formatting.The current
JSON.stringifyapproach provides full detail but produces verbose output. For better readability, consider formatting errors more concisely.Example alternative:
- let message = `[${this.name}]\nMessage:${this.message}\nStatus:${this.status}\nSerialized errors: ${this.errors.map( - e => JSON.stringify(e), - )}`; + let message = `[${this.name}]\nMessage:${this.message}\nStatus:${this.status}\nErrors: ${this.errors.map( + e => `[${e.code}] ${e.message}`, + ).join(', ')}`;
📜 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 (5)
packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx(0 hunks)packages/shared/src/errors/clerkApiError.ts(1 hunks)packages/shared/src/errors/clerkApiResponseError.ts(1 hunks)packages/shared/src/errors/clerkError.ts(1 hunks)packages/shared/src/errors/clerkRuntimeError.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/ui/components/SignUp/tests/SignUpContinue.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/shared/src/errors/clerkError.ts
- packages/shared/src/errors/clerkRuntimeError.ts
- packages/shared/src/errors/clerkApiError.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/shared/src/errors/clerkApiResponseError.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/shared/src/errors/clerkApiResponseError.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/errors/clerkApiResponseError.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/errors/clerkApiResponseError.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/shared/src/errors/clerkApiResponseError.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/shared/src/errors/clerkApiResponseError.ts
🧬 Code graph analysis (1)
packages/shared/src/errors/clerkApiResponseError.ts (4)
packages/shared/src/errors/clerkError.ts (2)
ClerkErrorParams(3-28)ClerkError(36-69)packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(11-25)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
⏰ 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 (1)
packages/shared/src/errors/clerkApiResponseError.ts (1)
45-56: Excellent type guard implementation.The type guard is well-documented with clear examples of both usage patterns (standalone function and method style). The implementation correctly uses the
createErrorTypeGuardutility.
| status: number; | ||
| clerkTraceId?: string; | ||
| retryAfter?: number; | ||
| errors: ClerkAPIError[]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make instance properties readonly for consistency with base class.
All instance properties in the base ClerkError class are readonly. These properties should follow the same pattern since they're set once in the constructor and shouldn't change afterward.
Apply this diff:
- status: number;
- clerkTraceId?: string;
- retryAfter?: number;
- errors: ClerkAPIError[];
+ readonly status: number;
+ readonly clerkTraceId?: string;
+ readonly retryAfter?: number;
+ readonly errors: ClerkAPIError[];Based on coding guidelines.
📝 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.
| status: number; | |
| clerkTraceId?: string; | |
| retryAfter?: number; | |
| errors: ClerkAPIError[]; | |
| readonly status: number; | |
| readonly clerkTraceId?: string; | |
| readonly retryAfter?: number; | |
| readonly errors: ClerkAPIError[]; |
🤖 Prompt for AI Agents
In packages/shared/src/errors/clerkApiResponseError.ts around lines 17 to 20,
the instance properties status, clerkTraceId, retryAfter, and errors should be
marked readonly to match the base ClerkError class; update their declarations to
use the readonly modifier so they are set only in the constructor and cannot be
reassigned.
0371d57 to
b788502
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/shared/src/errors/clerkApiResponseError.ts (1)
16-20: Make properties readonly for immutability.The
nameproperty and instance properties (status,clerkTraceId,retryAfter,errors) should be markedreadonlyto prevent modification after construction and align with theClerkErrorbase class pattern.Apply this diff:
- static name = 'ClerkAPIResponseError'; - status: number; - clerkTraceId?: string; - retryAfter?: number; - errors: ClerkAPIError[]; + static readonly name = 'ClerkAPIResponseError'; + readonly status: number; + readonly clerkTraceId?: string; + readonly retryAfter?: number; + readonly errors: ClerkAPIError[];Based on coding guidelines.
packages/shared/src/errors/clerkApiError.ts (1)
10-23: Consider if ClerkAPIError should extend Error.
ClerkAPIErrorimplements an interface but doesn't extendError. While this may be intentional if it's meant as a data holder for API error metadata (not a throwable exception), the "Error" naming suggests it should be throwable. Additionally:
- It lacks a stack trace for debugging
- Type guards using
instanceofwon't work in contexts expectingErrorinstances- Standard error handling code expects errors to extend
ErrorIf this is meant to be a data structure only, consider renaming to something like
ClerkApiErrorData. Otherwise, extendErroras flagged in previous reviews.Verify whether
ClerkAPIErrorinstances need to be thrown or are strictly used for data storage:#!/bin/bash # Search for ClerkAPIError being thrown or used in try-catch blocks rg -n -C3 --type=ts --type=tsx 'throw.*ClerkAPIError|catch.*ClerkAPIError'packages/shared/src/errors/clerkError.ts (1)
5-5: Fix typo in JSDoc comment."intented" should be "intended".
Apply this diff:
- * A message that describes the error. This is typically intented to be showed to the developers. + * A message that describes the error. This is typically intended to be showed to the developers.
📜 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 (8)
packages/clerk-js/src/core/resources/Verification.ts(2 hunks)packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx(0 hunks)packages/shared/src/error.ts(1 hunks)packages/shared/src/errors/clerkApiError.ts(1 hunks)packages/shared/src/errors/clerkApiResponseError.ts(1 hunks)packages/shared/src/errors/clerkError.ts(1 hunks)packages/shared/src/errors/clerkRuntimeError.ts(1 hunks)packages/shared/src/errors/parseError.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/ui/components/SignUp/tests/SignUpContinue.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/errors/clerkRuntimeError.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/shared/src/error.tspackages/shared/src/errors/parseError.tspackages/clerk-js/src/core/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkApiError.tspackages/shared/src/errors/clerkError.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/shared/src/error.tspackages/shared/src/errors/parseError.tspackages/clerk-js/src/core/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkApiError.tspackages/shared/src/errors/clerkError.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/error.tspackages/shared/src/errors/parseError.tspackages/clerk-js/src/core/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkApiError.tspackages/shared/src/errors/clerkError.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/error.tspackages/shared/src/errors/parseError.tspackages/clerk-js/src/core/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkApiError.tspackages/shared/src/errors/clerkError.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/shared/src/error.tspackages/shared/src/errors/parseError.tspackages/clerk-js/src/core/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkApiError.tspackages/shared/src/errors/clerkError.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/shared/src/error.tspackages/shared/src/errors/parseError.tspackages/clerk-js/src/core/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkApiError.tspackages/shared/src/errors/clerkError.ts
🧬 Code graph analysis (5)
packages/shared/src/errors/parseError.ts (2)
packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)
packages/clerk-js/src/core/resources/Verification.ts (1)
packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)
packages/shared/src/errors/clerkApiResponseError.ts (4)
packages/shared/src/errors/clerkError.ts (3)
ClerkErrorParams(3-28)ClerkError(36-70)name(44-46)packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
packages/shared/src/errors/clerkApiError.ts (2)
packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
packages/shared/src/errors/clerkError.ts (1)
packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
⏰ 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). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (15)
packages/clerk-js/src/core/resources/Verification.ts (2)
1-1: LGTM!The import changes align with the new error class hierarchy. The switch from
parseErrortoClerkAPIErrorconstructor is consistent with the deprecation noted inparseError.ts.
60-60: Verify ClerkAPIError usage aligns with requirements.The switch from
parseErrorto theClerkAPIErrorconstructor is consistent with the deprecation. However, note thatClerkAPIErrordoesn't extendError(as flagged inclerkApiError.tsreview), meaning it won't have a stack trace and cannot be thrown. Since this property appears to store API error metadata rather than a runtime exception, this may be acceptable.Ensure this design decision aligns with how
this.erroris used throughout the codebase.packages/shared/src/error.ts (1)
3-4: LGTM!The module path updates for
ClerkAPIError,ClerkAPIResponseError, andClerkRuntimeErrorare consistent with the error hierarchy refactoring. The public API surface remains unchanged.Also applies to: 12-12
packages/shared/src/errors/clerkApiResponseError.ts (3)
22-30: LGTM!The constructor properly initializes the base class, sets the prototype chain, and maps error data to
ClerkAPIErrorinstances.
32-42: LGTM!The
toStringmethod provides a comprehensive debug representation including all relevant error details.
44-48: LGTM!The
formatMessageoverride preserves backward compatibility, and the type guard follows the established pattern usingcreateErrorTypeGuard. Documentation is clear and includes usage examples.Also applies to: 50-61
packages/shared/src/errors/clerkApiError.ts (2)
25-40: LGTM!The
parseJsonErrormethod correctly transforms the JSON structure to the internal representation, handling the snake_case to camelCase conversion appropriately.
46-46: LGTM!The type guard follows the standard pattern using
createErrorTypeGuard.packages/shared/src/errors/parseError.ts (4)
1-3: LGTM!The import changes correctly separate the type interface from the class implementation, aligning with the new error hierarchy.
10-12: LGTM!The function correctly uses the
ClerkAPIErrorconstructor and returns the interface type for flexibility.
17-23: LGTM!The deprecation notice is clear and directs users to the new
ClerkAPIErrorclass. The implementation is consistent withparseErrors.
30-45: LGTM!The
errorToJSONfunction correctly handles serialization, including null cases and proper field mapping.packages/shared/src/errors/clerkError.ts (3)
30-35: Temporary development flag noted.The
__DEV__flag is documented as a temporary placeholder pending a build-time DCE (Dead Code Elimination) implementation. This is acceptable for the current phase.
48-55: LGTM!The constructor properly initializes all properties including
longMessageandcause. The prototype chain is correctly set up.
72-79: LGTM!The type guard implementation is appropriately defensive for a base error class, combining both
instanceofchecks and theclerkErrorproperty check.
nikosdouvlis
force-pushed
the
nikos/userland-errors-p-6
branch
from
b788502 to
2fef305
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/shared/src/errors/clerkApiResponseError.ts (1)
17-21: Mark instance fields readonly for immutability and consistencyApply:
- status: number; - clerkTraceId?: string; - retryAfter?: number; - errors: ClerkAPIError[]; + readonly status: number; + readonly clerkTraceId?: string; + readonly retryAfter?: number; + readonly errors: ClerkAPIError[];As per coding guidelines (prefer readonly for immutable data).
packages/shared/src/errors/clerkError.ts (1)
5-5: Fix JSDoc typo: "intented" → "intended"- * A message that describes the error. This is typically intented to be showed to the developers. + * A message that describes the error. This is typically intended to be showed to the developers.
🧹 Nitpick comments (6)
packages/clerk-js/src/core/resources/Verification.ts (1)
33-34: Prefer interface type for cross-package stabilityTyping the property and serializer against the public interface helps avoid tight coupling to the class.
Apply:
-import { ClerkAPIError, errorToJSON } from '@clerk/shared/error'; +import { ClerkAPIError, errorToJSON } from '@clerk/shared/error'; +import type { ClerkAPIError as ClerkAPIErrorInterface } from '@clerk/types' - error: ClerkAPIError | null = null; + error: ClerkAPIErrorInterface | null = null;No runtime change; improves API boundary clarity. Please verify the interface export exists in @clerk/types.
Also applies to: 60-60, 77-77
packages/shared/src/errors/clerkApiResponseError.ts (1)
32-41: Improve errors serialization in toStringSerialize the entire array as JSON for clearer logs; include retryAfter when present.
- public toString() { - let message = `[${this.name}]\nMessage:${this.message}\nStatus:${this.status}\nSerialized errors: ${this.errors.map( - e => JSON.stringify(e), - )}`; - - if (this.clerkTraceId) { - message += `\nClerk Trace ID: ${this.clerkTraceId}`; - } - - return message; - } + public toString() { + const serializedErrors = JSON.stringify(this.errors); + let message = `[${this.name}]\nMessage:${this.message}\nStatus:${this.status}\nSerialized errors:${serializedErrors}`; + if (this.retryAfter != null) { + message += `\nRetry-After:${this.retryAfter}`; + } + if (this.clerkTraceId) { + message += `\nClerk Trace ID: ${this.clerkTraceId}`; + } + return message; + }packages/shared/src/errors/parseError.ts (2)
10-12: Simplify: map is sufficient on empty arrays-export function parseErrors(data: ClerkAPIErrorJSON[] = []): ClerkAPIErrorInterface[] { - return data.length > 0 ? data.map(e => new ClerkAPIError(e)) : []; -} +export function parseErrors(data: ClerkAPIErrorJSON[] = []): ClerkAPIErrorInterface[] { + return data.map(e => new ClerkAPIError(e)); +}
30-45: Broaden parameter type to the public interfaceLet errorToJSON accept the interface to avoid coupling to the class and support callers that only have interface-typed values.
-export function errorToJSON(error: ClerkAPIError | null): ClerkAPIErrorJSON { +export function errorToJSON(error: ClerkAPIErrorInterface | null): ClerkAPIErrorJSON { return { code: error?.code || '', message: error?.message || '', long_message: error?.longMessage, meta: { param_name: error?.meta?.paramName, session_id: error?.meta?.sessionId, email_addresses: error?.meta?.emailAddresses, identifiers: error?.meta?.identifiers, zxcvbn: error?.meta?.zxcvbn, plan: error?.meta?.plan, is_plan_upgrade_possible: error?.meta?.isPlanUpgradePossible, }, }; }As per coding guidelines (prefer interfaces for extensible shapes).
packages/shared/src/errors/clerkError.ts (2)
61-69: Anchor the prefix-stripping regex to the startAvoid removing "Clerk:" that appears elsewhere in the message.
- const prefix = 'Clerk:'; - const regex = new RegExp(prefix.replace(' ', '\\s*'), 'i'); - msg = msg.replace(regex, ''); + const prefix = 'Clerk:'; + msg = msg.replace(/^\s*Clerk:\s*/i, ''); msg = `${prefix} ${msg.trim()}\n\n(code="${code}")\n\n`;
34-35: Replace hardcoded DEV with a build-time flag (follow-up)The constant prevents DCE. Consider a build-time flag (e.g., process.env.NODE_ENV !== 'production') to strip docs in prod.
Would you like a follow-up PR to wire a tree-shakable dev flag across packages?
Also applies to: 69-73
📜 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 (9)
packages/clerk-js/src/core/resources/Verification.ts(2 hunks)packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx(0 hunks)packages/shared/src/__tests__/error.test.ts(1 hunks)packages/shared/src/error.ts(1 hunks)packages/shared/src/errors/clerkApiError.ts(1 hunks)packages/shared/src/errors/clerkApiResponseError.ts(1 hunks)packages/shared/src/errors/clerkError.ts(1 hunks)packages/shared/src/errors/clerkRuntimeError.ts(1 hunks)packages/shared/src/errors/parseError.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/ui/components/SignUp/tests/SignUpContinue.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/shared/src/error.ts
- packages/shared/src/errors/clerkRuntimeError.ts
- packages/shared/src/errors/clerkApiError.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/resources/Verification.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/errors/clerkError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/shared/src/__tests__/error.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/shared/src/__tests__/error.test.ts
🧬 Code graph analysis (5)
packages/clerk-js/src/core/resources/Verification.ts (2)
packages/shared/src/error.ts (1)
ClerkAPIError(3-3)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)
packages/shared/src/errors/clerkApiResponseError.ts (4)
packages/shared/src/errors/clerkError.ts (3)
ClerkErrorParams(3-28)ClerkError(36-74)name(44-46)packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
packages/shared/src/errors/clerkError.ts (1)
packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
packages/shared/src/__tests__/error.test.ts (1)
packages/shared/src/errors/clerkRuntimeError.ts (1)
ClerkRuntimeError(15-26)
packages/shared/src/errors/parseError.ts (2)
packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)
⏰ 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 (2)
packages/shared/src/__tests__/error.test.ts (2)
42-42: LGTM: validates new "Clerk:" formatting and duplicate-prefix strippingBoth cases correctly assert the normalized message format and avoid double prefixes. Solid coverage.
Also applies to: 47-49
54-57: LGTM: properties and toString assertions match the new base class behaviorName, code, message, and toString checks are accurate for ClerkRuntimeError.
| } | ||
|
|
||
| export class ClerkAPIResponseError extends ClerkError implements ClerkAPIResponseErrorInterface { | ||
| static name = 'ClerkAPIResponseError'; |
There was a problem hiding this comment.
Remove static name assignment; it can throw at runtime
Assigning to Function.name is non-writable in JS and can throw. Rely on the base getter returning constructor.name.
Apply:
- static name = 'ClerkAPIResponseError';
+ // Name resolves from constructor via base class getter.📝 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.
| static name = 'ClerkAPIResponseError'; | |
| // Name resolves from constructor via base class getter. |
🤖 Prompt for AI Agents
In packages/shared/src/errors/clerkApiResponseError.ts around line 16, remove
the static name = 'ClerkAPIResponseError' assignment because writing to
Function.name can throw at runtime; instead delete that line and rely on the
default Error/constructor.name behavior (or, if a stable name is required, set a
readonly instance property in the constructor) so the class no longer attempts
to assign to Function.name.
| const __DEV__ = true; | ||
|
|
||
| export class ClerkError extends Error { | ||
| static name = 'ClerkError'; |
There was a problem hiding this comment.
Remove static name assignment; it can throw at runtime
Assigning to Function.name is non-writable and may throw. The base getter returns constructor.name already.
- static name = 'ClerkError';
+ // Name resolves from constructor via getter; avoid static assignment.📝 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.
| static name = 'ClerkError'; | |
| // Name resolves from constructor via getter; avoid static assignment. |
🤖 Prompt for AI Agents
In packages/shared/src/errors/clerkError.ts around line 37, remove the static
name = 'ClerkError' assignment because Function.name is non-writable and can
throw at runtime; simply delete that line and rely on the default
constructor.name provided by the base Error/getter, ensuring no further changes
are needed to preserve the error class identity.
| export function isClerkError(val: unknown): val is ClerkError { | ||
| const typeguard = createErrorTypeGuard(ClerkError); | ||
| // Ths is the base error so we're being more defensive about the type guard | ||
| return typeguard(val) || (!!val && typeof val === 'object' && 'clerkError' in val && val.clerkError === true); | ||
| } |
There was a problem hiding this comment.
Make the fallback type guard type-safe
Avoid property access on unknown; narrow safely.
-export function isClerkError(val: unknown): val is ClerkError {
- const typeguard = createErrorTypeGuard(ClerkError);
- // Ths is the base error so we're being more defensive about the type guard
- return typeguard(val) || (!!val && typeof val === 'object' && 'clerkError' in val && val.clerkError === true);
-}
+export function isClerkError(val: unknown): val is ClerkError {
+ const typeguard = createErrorTypeGuard(ClerkError);
+ if (typeguard(val)) return true;
+ if (typeof val === 'object' && val !== null) {
+ const v = val as { clerkError?: unknown };
+ return v.clerkError === true;
+ }
+ return false;
+}Based on coding guidelines (proper type annotations; avoid unsafe property access).
📝 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.
| export function isClerkError(val: unknown): val is ClerkError { | |
| const typeguard = createErrorTypeGuard(ClerkError); | |
| // Ths is the base error so we're being more defensive about the type guard | |
| return typeguard(val) || (!!val && typeof val === 'object' && 'clerkError' in val && val.clerkError === true); | |
| } | |
| export function isClerkError(val: unknown): val is ClerkError { | |
| const typeguard = createErrorTypeGuard(ClerkError); | |
| if (typeguard(val)) { | |
| return true; | |
| } | |
| if (typeof val === 'object' && val !== null) { | |
| const v = val as { clerkError?: unknown }; | |
| return v.clerkError === true; | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In packages/shared/src/errors/clerkError.ts around lines 79 to 83, the fallback
type guard accesses properties on an unknown value unsafely; change the guard to
first narrow val to a non-null object and then access the property with a typed
index (e.g. if (val && typeof val === 'object' && 'clerkError' in val) { const v
= val as Record<string, unknown>; return v['clerkError'] === true } ), combining
that check with the existing createErrorTypeGuard result so the function remains
type-safe and avoids direct property access on unknown.
2fef305 to
8662db6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/shared/src/errors/clerkApiResponseError.ts (2)
17-20: Make properties readonly to match baseClerkErrorpatternThese are set once in the constructor and should not change.
- status: number; - clerkTraceId?: string; - retryAfter?: number; - errors: ClerkAPIError[]; + readonly status: number; + readonly clerkTraceId?: string; + readonly retryAfter?: number; + readonly errors: ClerkAPIError[];
16-16: Remove static name assignment; rely onconstructor.nameStatic assignment to
Function.nameis non‑writable and may throw. The base getter already returnsconstructor.name.- static name = 'ClerkAPIResponseError'; + // Name resolves from constructor via base class getter.
🧹 Nitpick comments (6)
packages/shared/src/errors/clerkRuntimeError.ts (3)
20-20: Optional: mark propertypublicfor explicit API visibilityFor public APIs, our guidelines prefer explicit access modifiers.
[As per coding guidelines]- readonly clerkRuntimeError = true as const; + public readonly clerkRuntimeError = true as const;
7-14: Enhance JSDoc: add parameter tags for public APIDocument constructor params for better DX and to satisfy public API docs.
[As per coding guidelines]/** * Custom error class for representing Clerk runtime errors. * * @class ClerkRuntimeError * * @example * throw new ClerkRuntimeError('An error occurred', { code: 'password_invalid' }); + * @param message A developer‑facing description of the error. + * @param options Additional metadata like `code`, `longMessage`, `docsUrl`, and optional `cause`. */
36-38: Clarify method‑style usage in exampleThe guard isn’t attached by default. Show the actual method‑style call pattern.
- * // As a method (when attached to error object) - * if (error.isClerkRuntimeError()) { ... } + * // As a method (only if you attach it to the instance) + * error.isClerkRuntimeError = isClerkRuntimeError; + * if (error.isClerkRuntimeError()) { ... }packages/shared/src/errors/clerkApiResponseError.ts (2)
32-42: Add explicit return type andoverridetotoStringConforms to our TS guidelines and indicates intentional override.
[As per coding guidelines]- public toString() { + public override toString(): string { let message = `[${this.name}]\nMessage:${this.message}\nStatus:${this.status}\nSerialized errors: ${this.errors.map( e => JSON.stringify(e), )}`; if (this.clerkTraceId) { message += `\nClerk Trace ID: ${this.clerkTraceId}`; } return message; }
61-61: Nit: considerisClerkAPIResponseErrorfor naming consistencyThe class uses “API” capitalization; matching the guard name is slightly clearer. Non‑blocking.
packages/shared/src/errors/parseError.ts (1)
30-45: Accept interface type inerrorToJSONto avoid coupling to classThis widens usability (callers with only the interface don’t need the class type).
-export function errorToJSON(error: ClerkAPIError | null): ClerkAPIErrorJSON { +export function errorToJSON(error: ClerkAPIErrorInterface | null | undefined): ClerkAPIErrorJSON { return { code: error?.code || '', message: error?.message || '', long_message: error?.longMessage, meta: { param_name: error?.meta?.paramName, session_id: error?.meta?.sessionId, email_addresses: error?.meta?.emailAddresses, identifiers: error?.meta?.identifiers, zxcvbn: error?.meta?.zxcvbn, plan: error?.meta?.plan, is_plan_upgrade_possible: error?.meta?.isPlanUpgradePossible, }, }; }
📜 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 ignored due to path filters (1)
.typedoc/__tests__/__snapshots__/file-structure.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
.typedoc/__tests__/file-structure.test.ts(0 hunks)packages/clerk-js/src/core/resources/Verification.ts(2 hunks)packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx(0 hunks)packages/shared/src/__tests__/error.test.ts(1 hunks)packages/shared/src/error.ts(1 hunks)packages/shared/src/errors/clerkApiError.ts(1 hunks)packages/shared/src/errors/clerkApiResponseError.ts(1 hunks)packages/shared/src/errors/clerkError.ts(1 hunks)packages/shared/src/errors/clerkRuntimeError.ts(1 hunks)packages/shared/src/errors/parseError.ts(1 hunks)
💤 Files with no reviewable changes (2)
- .typedoc/tests/file-structure.test.ts
- packages/clerk-js/src/ui/components/SignUp/tests/SignUpContinue.test.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/shared/src/error.ts
- packages/shared/src/errors/clerkApiError.ts
- packages/shared/src/errors/clerkError.ts
- packages/clerk-js/src/core/resources/Verification.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/shared/src/errors/clerkRuntimeError.tspackages/shared/src/errors/clerkApiResponseError.tspackages/shared/src/__tests__/error.test.tspackages/shared/src/errors/parseError.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/shared/src/__tests__/error.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/shared/src/__tests__/error.test.ts
🧬 Code graph analysis (4)
packages/shared/src/errors/clerkRuntimeError.ts (3)
packages/shared/src/errors/clerkError.ts (2)
ClerkErrorParams(3-28)ClerkError(36-74)packages/shared/src/errors/helpers.ts (1)
isClerkRuntimeError(83-85)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
packages/shared/src/errors/clerkApiResponseError.ts (4)
packages/shared/src/errors/clerkError.ts (3)
ClerkErrorParams(3-28)ClerkError(36-74)name(44-46)packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)packages/shared/src/errors/createErrorTypeGuard.ts (1)
createErrorTypeGuard(21-39)
packages/shared/src/__tests__/error.test.ts (1)
packages/shared/src/errors/clerkRuntimeError.ts (1)
ClerkRuntimeError(15-26)
packages/shared/src/errors/parseError.ts (2)
packages/types/src/errors.ts (1)
ClerkAPIErrorJSON(1-25)packages/shared/src/errors/clerkApiError.ts (1)
ClerkAPIError(10-41)
⏰ 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 (sessions:staging, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- 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/shared/src/__tests__/error.test.ts (1)
39-49: Tests updated to new "Clerk:" prefix — looks goodAssertions now match the centralized formatting and guard behavior.
Also applies to: 51-57
| * throw new ClerkRuntimeError('An error occurred', { code: 'password_invalid' }); | ||
| */ | ||
| export class ClerkRuntimeError extends ClerkError { | ||
| static name = 'ClerkRuntimeError'; |
There was a problem hiding this comment.
Remove static name assignment; Function.name is non‑writable and can throw
Rely on the base getter using constructor.name. Avoid assigning to Function.name in strict mode.
- static name = 'ClerkRuntimeError';
+ // Name resolves from constructor via base class getter.📝 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.
| static name = 'ClerkRuntimeError'; | |
| // Name resolves from constructor via base class getter. |
🤖 Prompt for AI Agents
In packages/shared/src/errors/clerkRuntimeError.ts around line 16, remove the
static name = 'ClerkRuntimeError'; assignment because Function.name is
non-writable in strict mode; rely on the base error class/getter that uses
constructor.name instead and delete this assignment (and any tests or code that
expect a writable Function.name) so the runtime won't throw when the class is
evaluated.
| * // As a method (when attached to error object) | ||
| * if (error.isClerkRuntimeError()) { ... } | ||
| */ | ||
| export const isClerkRuntimeError = createErrorTypeGuard(ClerkRuntimeError); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Avoid duplicate guards; ensure a single exported isClerkRuntimeError
There’s another helper with the same name relying on a marker prop (helpers.ts snippet). Having two exports with the same name can cause conflicts or confusion. Pick one (prefer the factory instanceof guard) and deprecate the other, or re‑export only one from the public surface.
Run to locate duplicates and re‑exports:
🏁 Script executed:
#!/bin/bash
# Find all definitions/exports named isClerkRuntimeError
rg -nP '\b(export\s+(const|function)\s+isClerkRuntimeError\b|export\s*\{[^\}]*\bisClerkRuntimeError\b[^\}]*\})' -C2
# Check aggregator exports
rg -nP 'from\s+["'\''][\.\/].*error' -C2 packages/shared/src/error.ts || true
rg -nP '\bisClerkRuntimeError\b' -C3 packages/shared/src/error.ts || trueLength of output: 1500
Eliminate duplicate isClerkRuntimeError guards
Drop the marker-prop guard in packages/shared/src/errors/helpers.ts (and its re-export in packages/shared/src/error.ts), exposing only the factory-based guard in packages/shared/src/errors/clerkRuntimeError.ts.
🤖 Prompt for AI Agents
In packages/shared/src/errors/clerkRuntimeError.ts around line 39, keep the
factory-based guard export (export const isClerkRuntimeError =
createErrorTypeGuard(ClerkRuntimeError)); and remove the duplicate marker-prop
guard: delete the marker-based isClerkRuntimeError from
packages/shared/src/errors/helpers.ts and remove its re-export from
packages/shared/src/error.ts; update any imports across the codebase that
referenced the helpers' or error.ts re-export to import the factory-based guard
from packages/shared/src/errors/clerkRuntimeError.ts instead, ensuring only the
factory-based implementation is exported and used.
3 participants
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit