refactor: isORPCErrorStatus -> ORPCError.isValidStatus#339
refactor: isORPCErrorStatus -> ORPCError.isValidStatus#339
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes refactor error handling across multiple modules by replacing the standalone function Changes
Sequence Diagram(s)sequenceDiagram
participant Decoder as Decoder (e.g., rpc-link-codec.ts)
participant ORPCError as ORPCError
participant ErrorHandler as Error Handler
Decoder->>ORPCError: isValidStatus(response.status)
alt Status Valid
ORPCError-->>Decoder: true
Decoder->>Decoder: Process response normally
else Status Invalid
ORPCError-->>Decoder: false
Decoder->>ErrorHandler: Throw error using ORPCError
end
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/openapi
@orpc/openapi-client
@orpc/react
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/svelte-query
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/contract/src/procedure.ts (1)
33-33: Consider using optional chaining for cleaner code.The static analysis tool suggested that this line would benefit from optional chaining for better readability and to prevent potential errors.
- if (Object.values(def.errorMap).some(val => val && val.status && !ORPCError.isValidStatus(val.status))) { + if (Object.values(def.errorMap).some(val => val?.status && !ORPCError.isValidStatus(val.status))) {🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/client/src/adapters/standard/rpc-link-codec.test.ts(2 hunks)packages/client/src/adapters/standard/rpc-link-codec.ts(2 hunks)packages/client/src/error.test.ts(2 hunks)packages/client/src/error.ts(3 hunks)packages/contract/src/procedure.test.ts(1 hunks)packages/contract/src/procedure.ts(2 hunks)packages/openapi-client/src/adapters/standard/openapi-link-codec.test.ts(3 hunks)packages/openapi-client/src/adapters/standard/openapi-link-codec.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
packages/contract/src/procedure.ts (3)
packages/client/src/error.ts (1)
ORPCError(100-165)packages/server/src/index.ts (1)
ORPCError(23-23)packages/contract/src/index.ts (1)
ORPCError(17-17)
packages/client/src/error.test.ts (1)
packages/client/src/error.ts (1)
ORPCError(100-165)
packages/openapi-client/src/adapters/standard/openapi-link-codec.ts (1)
packages/client/src/error.ts (1)
ORPCError(100-165)
packages/client/src/adapters/standard/rpc-link-codec.ts (1)
packages/client/src/error.ts (1)
ORPCError(100-165)
🪛 Biome (1.9.4)
packages/contract/src/procedure.ts
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (19)
packages/client/src/error.test.ts (3)
1-1: LGTM! Import statement updated to reflect refactoring.The import statement has been correctly updated to import
ORPCErrordirectly, removing the now-deletedisORPCErrorStatusfunction reference.
83-83: Good addition - comprehensive test coverage for invalid status in JSON.This new test case ensures proper validation when a JSON object contains an invalid status code. This provides good complementary coverage to the
isValidStatustest.
87-94: LGTM! Comprehensive test coverage for the new static method.The new test case for
isValidStatusproperly verifies the behavior of the method with multiple status code inputs, ensuring it correctly identifies valid error status codes (< 200 or >= 400) and rejects invalid ones.packages/client/src/error.ts (3)
107-107: LGTM! Correctly updated to use the new static method.The constructor now correctly references the static
isValidStatusmethod instead of the previous standalone function.
131-133: Good implementation of the static method.The method correctly implements the same logic as the previous standalone function. This refactoring improves encapsulation by keeping the status validation logic within the
ORPCErrorclass.
161-161: LGTM! Updated reference to the static method.The
isValidJSONmethod now correctly calls the staticisValidStatusmethod.packages/contract/src/procedure.test.ts (2)
1-1: LGTM! Updated import to directly reference ORPCError.The import statement has been correctly updated to import
ORPCErrordirectly rather than the entire ClientModule.
5-5: LGTM! Spy correctly updated to reference the new static method.The spy has been properly updated to target
ORPCError.isValidStatusinstead of the previous standalone function.packages/contract/src/procedure.ts (2)
5-5: LGTM! Updated import to reference ORPCError directly.The import statement has been correctly updated to import
ORPCErrorfrom '@orpc/client'.
29-29: LGTM! Updated to use static method.The conditional check now correctly uses
ORPCError.isValidStatusto validate the route's success status.packages/openapi-client/src/adapters/standard/openapi-link-codec.ts (1)
191-191: Good refactoring to use ORPCError.isValidStatusThe change from using the standalone function
isORPCErrorStatusto the static methodORPCError.isValidStatusis a good encapsulation of error handling logic within theORPCErrorclass.packages/client/src/adapters/standard/rpc-link-codec.ts (2)
7-7: Clean up importsGood update to directly import
ORPCErrorfrom the error module instead of importing the standalone function.
108-108: Improved error handling approachUsing
ORPCError.isValidStatusinstead of a standalone function provides better encapsulation of error handling logic within theORPCErrorclass while maintaining the same functionality.packages/client/src/adapters/standard/rpc-link-codec.test.ts (3)
2-2: Updated import to match implementation changesCorrectly updated the import to directly import
ORPCErrorinstead of the entire ErrorModule, which aligns with the implementation changes.
8-8: Updated test spy to reflect implementation changeThe spy has been correctly updated to target
ORPCError.isValidStatusinstead of the previous standalone function.
189-189: Test assertion updated for flexibilityChanged from checking the exact number of calls to simply verifying the method was called, which provides more flexibility for future modifications.
packages/openapi-client/src/adapters/standard/openapi-link-codec.test.ts (3)
1-1: Updated import statementCorrectly updated the import to directly import
ORPCErrorfrom '@orpc/client', aligning with the implementation changes.
10-10: Updated test spy to reflect implementation changeThe spy is now correctly targeting
ORPCError.isValidStatusinstead of the previous standalone function.
380-380: More flexible test assertionChanged from checking the exact number of calls to simply verifying the method was called, which provides more flexibility while still ensuring the expected behavior.
|
I don't like this, I think we should migrate out of static methods |
1 participant
Summary by CodeRabbit
Refactor
Tests