Handle ErrClientNotActivated and ErrClientNotFound #865
Conversation
WalkthroughThis update primarily focuses on enhancing error handling and updating package versions. It involves refactoring error codes for consistency, improving the functionality of error-related modules, and incorporating custom matchers for ViTest to validate error codes. Package versions have been updated to ensure up-to-date dependencies, and some minor tweaks have been made to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #865 +/- ##
==========================================
- Coverage 80.91% 80.84% -0.07%
==========================================
Files 60 60
Lines 4615 4637 +22
Branches 937 942 +5
==========================================
+ Hits 3734 3749 +15
- Misses 613 620 +7
Partials 268 268 ☔ View full report in Codecov by Sentry. |
hackerwins
changed the title
hackerwins
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (13)
- .eslintignore (1 hunks)
- package.json (2 hunks)
- src/api/converter.ts (10 hunks)
- src/client/client.ts (19 hunks)
- src/document/crdt/counter.ts (3 hunks)
- src/document/crdt/primitive.ts (2 hunks)
- src/document/document.ts (2 hunks)
- src/document/json/object.ts (2 hunks)
- src/util/error.ts (1 hunks)
- test/integration/client_test.ts (6 hunks)
- test/vitest.d.ts (1 hunks)
- test/vitest.setup.ts (1 hunks)
- vitest.config.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- .eslintignore
- src/util/error.ts
- vitest.config.ts
Additional comments not posted (41)
test/vitest.setup.ts (1)
1-33: LGTM! Custom matcher toThrowErrorCode.The custom matcher
toThrowErrorCodeis correctly implemented and useful for validating error codes in tests.test/vitest.d.ts (1)
1-31: LGTM! Type definitions for custom matcher toThrowErrorCode.The type definitions for the custom matcher
toThrowErrorCodeare correct and comprehensive.package.json (2)
43-43: LGTM! Dependency update for @buf/googleapis_googleapis.connectrpc_es.The version update to
^1.4.0-20240524201209-f0e53af8f2fc.3is correct.
46-46: LGTM! Dependency update for @connectrpc/protoc-gen-connect-es.The version update to
^1.4.0is correct.src/document/json/object.ts (2)
150-150: LGTM! Improved error handling in setInternal.The error code update to
Code.ErrInvalidObjectKeyimproves error handling.
186-186: LGTM! Improved error handling in buildObjectMembers.The error code update to
Code.ErrInvalidObjectKeyimproves error handling.src/document/crdt/counter.ts (3)
73-75: LGTM! Consistent error handling.The use of
Code.ErrUnimplementedfor unimplemented types is consistent with the new error handling approach.
103-105: LGTM! Consistent error handling.The use of
Code.ErrUnimplementedfor unimplemented types invalueFromBytesis consistent with the new error handling approach.
232-234: LGTM! Consistent error handling.The use of
Code.ErrUnimplementedfor unimplemented types intoBytesis consistent with the new error handling approach.src/document/crdt/primitive.ts (1)
95-97: LGTM! Consistent error handling.The use of
Code.ErrUnimplementedfor unimplemented types invalueFromBytesis consistent with the new error handling approach.src/client/client.ts (20)
249-251: LGTM! Improved error handling inactivatemethod.The use of
handleConnectErrorimproves error handling consistency.
275-277: LGTM! Improved error handling indeactivatemethod.The use of
handleConnectErrorimproves error handling consistency.
293-296: LGTM! Improved error handling inattachmethod.The use of
Code.ErrClientNotActivatedandCode.ErrDocumentNotDetachedimproves error handling consistency.
300-302: LGTM! Improved error handling inattachmethod.The use of
Code.ErrDocumentNotDetachedimproves error handling consistency.
365-368: LGTM! Improved error handling indetachmethod.The use of
Code.ErrClientNotActivatedimproves error handling consistency.
373-375: LGTM! Improved error handling indetachmethod.The use of
Code.ErrDocumentNotAttachedimproves error handling consistency.
417-420: LGTM! Improved error handling inchangeSyncModemethod.The use of
Code.ErrClientNotActivatedimproves error handling consistency.
426-428: LGTM! Improved error handling inchangeSyncModemethod.The use of
Code.ErrDocumentNotAttachedimproves error handling consistency.
469-472: LGTM! Improved error handling insyncmethod.The use of
Code.ErrClientNotActivatedimproves error handling consistency.
479-481: LGTM! Improved error handling insyncmethod.The use of
Code.ErrDocumentNotAttachedimproves error handling consistency.
483-489: LGTM! Improved error handling insyncmethod.The use of
handleConnectErrorimproves error handling consistency.
497-501: LGTM! Improved error handling insyncmethod.The use of
handleConnectErrorimproves error handling consistency.
510-513: LGTM! Improved error handling inremovemethod.The use of
Code.ErrClientNotActivatedimproves error handling consistency.
518-520: LGTM! Improved error handling inremovemethod.The use of
Code.ErrDocumentNotAttachedimproves error handling consistency.
546-548: LGTM! Improved error handling inremovemethod.The use of
handleConnectErrorimproves error handling consistency.
612-614: LGTM! Improved error handling inrunSyncLoopmethod.The use of
handleConnectErrorimproves error handling consistency.
633-635: LGTM! Improved error handling inrunWatchLoopmethod.The use of
Code.ErrDocumentNotAttachedimproves error handling consistency.
644-647: LGTM! Improved error handling inrunWatchLoopmethod.The use of
Code.ErrClientNotActivatedimproves error handling consistency.
819-822: LGTM! Improved error handling inhandleConnectErrormethod.The method now correctly handles
Code.ErrClientNotActivatedandCode.ErrClientNotFound.
731-738: LGTM! Improved client state management indeactivateInternalmethod.The method correctly updates the client status to
Deactivatedand detaches documents.test/integration/client_test.ts (8)
66-81: LGTM! Comprehensive error handling tests inCan attach/detach document.The test case correctly checks for
Code.ErrDocumentNotDetachedandCode.ErrDocumentNotAttached.
Line range hint
820-833: LGTM! Comprehensive retry logic tests inShould retry on network failure and eventually succeed.The test case correctly checks for
ConnectErrorwith different error codes and verifies client state management.
Line range hint
753-780: LGTM! Comprehensive sync logic tests inShould apply previous changes when switching to realtime sync.The test case correctly checks for applying changes made while in manual sync mode.
Line range hint
781-815: LGTM! Comprehensive sync logic tests inShould not include changes applied in push-only mode when switching to realtime sync.The test case correctly checks for excluding changes made in push-only mode.
Line range hint
816-848: LGTM! Comprehensive sync logic tests inShould prevent remote changes in push-only mode.The test case correctly checks for preventing remote changes in push-only mode.
Line range hint
849-881: LGTM! Comprehensive sync logic tests inShould prevent remote changes in sync-off mode.The test case correctly checks for preventing remote changes in sync-off mode.
Line range hint
882-902: LGTM! Comprehensive sync logic tests inShould avoid unnecessary syncs in push-only mode.The test case correctly checks for avoiding unnecessary syncs in push-only mode.
798-800: LGTM! Comprehensive request handling tests inShould handle each request one by one.The test case correctly checks for handling requests sequentially.
src/api/converter.ts (1)
785-785: Consider specifying the return type asCode.The function returns a string, which might not be the expected type. Ensure the return type matches the expected
Codeenum.- export function errorCodeOf(error: ConnectError): string { + export function errorCodeOf(error: ConnectError): Code {src/document/document.ts (2)
630-630: Correctly handleErrDocumentRemovederror.The addition of the error handling for
ErrDocumentRemovedensures that the document update operation is prevented when the document is removed. This aligns with the expected behavior.
1592-1595: Correctly handleErrInvalidArgumenterror.The addition of the error handling for
ErrInvalidArgumentensures that thegetValueByPathmethod correctly validates the path argument, preventing invalid paths from causing issues.
What this PR does / why we need it?
Handle ErrClientNotActivated and ErrClientNotFound
This commit addresses detailed logic errors returned in RPC Error
response from the server, specifically
ErrClientNotActivatedandErrClientNotFound. It checks for these errors in the RPC responseand handles them by changing the Client's state.
In case of
ErrClientNotActivatedandErrClientNotFound, Client'sstate is changed to 'Deactivated,' and attached
Documents arechanged to
Detached. Therefore, if the application is subscribingthe status event using
Document.Subscribe, it will receiveDetached.Any background context you want to provide?
What are the relevant tickets?
Addresses yorkie-team/yorkie#928
Related to yorkie-team/yorkie#927
Checklist
Summary by CodeRabbit
New Features
vitesttesting framework.Bug Fixes
Chores
test/vitest.d.tsto the.eslintignorefile.Refactor