Fix deserialization issue with drizzle cursor #143
Conversation
|
Warning Rate limit exceeded@fracek has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 58 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis pull request introduces prerelease metadata for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StateManager as getState
participant CursorNormalizer as normalizeCursor
participant Database
Client->>StateManager: Request state data
StateManager->>CursorNormalizer: Normalize cursor input
CursorNormalizer-->>StateManager: Return normalized cursor
StateManager->>Database: Query using normalized cursor
Database-->>StateManager: Return query result
StateManager-->>Client: Deliver state data
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
fracek
force-pushed
the
fix-cursor-serde
branch
from
81f6df9 to
8e68be2
Compare
![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/plugin-drizzle/src/persistence.ts (1)
146-185: Consider improving error handling for cursor validation.The persistState function should validate the cursor before persisting it to ensure data consistency.
Consider adding cursor validation:
if (endCursor) { + const normalizedCursor = normalizeCursor({ + orderKey: endCursor.orderKey, + uniqueKey: endCursor.uniqueKey, + }); await tx .insert(checkpoints) .values({ id: indexerId, - orderKey: Number(endCursor.orderKey), - uniqueKey: endCursor.uniqueKey, + orderKey: Number(normalizedCursor.orderKey), + uniqueKey: normalizedCursor.uniqueKey, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
change/@apibara-plugin-drizzle-c4aaf5bc-24c6-4883-9b29-cf614279c5db.json(1 hunks)package.json(1 hunks)packages/beaconchain/package.json(1 hunks)packages/evm/package.json(1 hunks)packages/plugin-drizzle/package.json(1 hunks)packages/plugin-drizzle/src/persistence.ts(4 hunks)packages/plugin-drizzle/tests/storage.test.ts(4 hunks)packages/plugin-mongo/src/persistence.ts(2 hunks)packages/plugin-sqlite/package.json(1 hunks)packages/plugin-sqlite/src/persistence.ts(2 hunks)packages/protocol/src/common.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/plugin-drizzle/package.json
- packages/plugin-sqlite/package.json
- packages/evm/package.json
- change/@apibara-plugin-drizzle-c4aaf5bc-24c6-4883-9b29-cf614279c5db.json
- package.json
- packages/beaconchain/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (11)
packages/protocol/src/common.ts (1)
71-93: LGTM! The cursor normalization logic is well-implemented.The function correctly handles the following cases:
- When uniqueKey is present and non-empty, it's cast to the required 0x-prefixed format
- When uniqueKey is null or empty, it's omitted from the cursor object
packages/plugin-mongo/src/persistence.ts (2)
7-7: LGTM! Type change improves null handling.The type change from optional to required with null makes the uniqueKey handling more explicit and type-safe.
101-104: LGTM! Cursor normalization is properly integrated.The implementation correctly uses normalizeCursor to ensure consistent cursor format across the application.
packages/plugin-sqlite/src/persistence.ts (1)
60-63: LGTM! Cursor normalization is well-implemented.The implementation correctly:
- Handles the case when unique_key is undefined
- Uses ternary operator for concise null checking
packages/plugin-drizzle/src/persistence.ts (2)
18-18: LGTM! Field definition is appropriately simplified.The uniqueKey field definition is correctly simplified to allow null values while maintaining type safety.
212-215: LGTM! Cursor normalization is properly integrated.The implementation correctly uses normalizeCursor to ensure consistent cursor format.
packages/plugin-drizzle/tests/storage.test.ts (5)
678-686: LGTM! Test correctly handles null uniqueKey in checkpoints.The test case properly verifies that the checkpoint's uniqueKey can be null, which aligns with the PR's objective of fixing deserialization issues with empty uniqueKey values.
844-852: LGTM! Test verifies filter persistence with null uniqueKey.The test case properly validates that filters are persisted correctly in factory mode while handling null uniqueKey in checkpoints.
1028-1036: LGTM! Test validates state invalidation with null uniqueKey.The test case properly verifies that state invalidation works correctly while handling null uniqueKey in checkpoints.
1204-1212: LGTM! Test verifies state finalization with null uniqueKey.The test case properly validates that state finalization works correctly while handling null uniqueKey in checkpoints.
19-1266: Test suite provides comprehensive coverage for uniqueKey handling.The test suite thoroughly validates the handling of null uniqueKey values across different scenarios:
- State persistence
- Filter persistence in factory mode
- State invalidation
- State finalization
This comprehensive test coverage ensures the PR's objective of fixing deserialization issues with empty uniqueKey values is properly addressed.
fracek
force-pushed
the
fix-cursor-serde
branch
from
8e68be2 to
b35e8df
Compare
fracek
force-pushed
the
fix-cursor-serde
branch
from
b35e8df to
b624d01
Compare
2 participants
If the stored cursor has an empty uniqueKey, it's serialized as an empty string. This is an issue later when deserializing since that's not a valid value for the uniqueKey.