-
Notifications
You must be signed in to change notification settings - Fork 11
plugin-mongo: fix errors on restart and retry #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis pull request simplifies and expands the build, test, and release workflows by removing directory exclusion filters. It introduces new files for MongoDB support including configuration, Docker Compose, and an indexer for Starknet. Additionally, the update removes specific apibara-related scripts from several CLI examples. In the plugin-mongo package, collection handling is modified to retrieve existing collections and the transaction process is updated with a new option disabling retryable writes. A new JSON file marks a prerelease addressing a connection issue for the @apibara/plugin-mongo package. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Build Command
participant Test Command
GitHub Actions->>Build Command: Trigger build (pnpm build)
Build Command->>Test Command: Trigger test (pnpm test:ci)
sequenceDiagram
participant Starknet Indexer
participant Blockchain
participant Logger
participant MongoDB
Starknet Indexer->>Blockchain: Connect to event stream
Blockchain-->>Starknet Indexer: Send block data
Starknet Indexer->>Logger: Log block header & transaction insertion
Starknet Indexer->>MongoDB: Insert blocks and transactions
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/cli-mongodb/indexers/starknet.indexer.js (2)
8-8: Consider externalizing MongoDB connection string.The MongoDB connection string contains hardcoded credentials. While this may be acceptable for an example, it would be better to use environment variables for the connection details.
- const mongodb = new MongoClient("mongodb://mongo:mongo@localhost:27017/"); + const mongoUri = process.env.MONGODB_URI || "mongodb://mongo:mongo@localhost:27017/"; + const mongodb = new MongoClient(mongoUri);
24-32: Transform function looks good but could handle errors better.The transform function properly inserts data into MongoDB, but it lacks error handling. Consider adding try/catch blocks to handle potential database errors gracefully.
async transform({ block: { header, transactions } }) { const logger = useLogger(); const mongo = useMongoStorage(); + try { await mongo.collection("blocks").insertOne(header); await mongo.collection("transactions").insertMany(transactions); logger.info(`Inserted block ${header.blockNumber}`); + } catch (error) { + logger.error(`Error processing block ${header.blockNumber}: ${error.message}`); + throw error; // Re-throw to let the indexer framework handle it + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/workflows/build.yml(1 hunks).github/workflows/release.yml(1 hunks)change/@apibara-plugin-mongo-8a313e7a-5b45-4ae6-9727-2fa720b6f46c.json(1 hunks)examples/cli-drizzle/package.json(0 hunks)examples/cli-instrumentation/package.json(1 hunks)examples/cli-js/package.json(0 hunks)examples/cli-mongodb/apibara.config.mjs(1 hunks)examples/cli-mongodb/docker-compose.yaml(1 hunks)examples/cli-mongodb/indexers/starknet.indexer.js(1 hunks)examples/cli-mongodb/package.json(1 hunks)packages/plugin-mongo/src/persistence.ts(1 hunks)packages/plugin-mongo/src/utils.ts(1 hunks)
💤 Files with no reviewable changes (2)
- examples/cli-js/package.json
- examples/cli-drizzle/package.json
🔇 Additional comments (9)
examples/cli-mongodb/apibara.config.mjs (1)
1-3: Clean and minimal configuration setupThis is a simple and effective configuration file that provides a basic template for MongoDB integration with Apibara. The empty configuration is a good starting point that can be expanded as needed.
change/@apibara-plugin-mongo-8a313e7a-5b45-4ae6-9727-2fa720b6f46c.json (1)
1-7: Appropriate change file documenting the PR fixesThis change file correctly documents the purpose of this PR - fixing connection errors and disabling retry functionality. The metadata is complete with package name, email, and the appropriate dependent change type.
packages/plugin-mongo/src/persistence.ts (1)
25-26: Good fix for the collection creation errorChanging from
createCollectiontocollectioneffectively addresses the error that occurs when trying to create collections that already exist during restart. This method retrieves the collection reference without attempting to create it, allowing the application to restart smoothly while maintaining the existing collections.examples/cli-mongodb/docker-compose.yaml (1)
1-20: MongoDB setup looks good with proper replica set configuration.The Docker Compose configuration properly sets up MongoDB with a replica set, which is necessary for transaction support. The configuration includes proper volume mounting for data persistence and appropriate environment variables for initialization.
examples/cli-instrumentation/package.json (1)
7-11: Removal of build and start scripts appears intentional.The
buildandstartscripts have been removed from the package.json file. This matches the PR's simplification of the build workflow and aligns with similar changes in other example files..github/workflows/build.yml (1)
46-51: Build workflow simplification looks good.The removal of directory exclusion filters from the build and test commands simplifies the workflow and ensures all directories are included in the build and test processes. This change aligns with the PR objective of improving the build process.
examples/cli-mongodb/indexers/starknet.indexer.js (1)
1-34: Note about the core PR objectives.This example file demonstrates the usage of the MongoDB plugin, but I don't see the core fixes mentioned in the PR description (handling collection creation errors and transaction retry behavior). These fixes likely exist in the
@apibara/plugin-mongopackage itself rather than this example.packages/plugin-mongo/src/utils.ts (1)
15-22:✅ Verification successful
Transaction retry behavior fixed
Adding the
retryWrites: falseoption to thesession.withTransactioncall properly addresses one of the PR objectives - preventing automatic transaction retries by MongoDB. This change is important because automatic retries can cause unexpected behavior when used with custom middleware.
🏁 Script executed:
#!/bin/bash # Check MongoDB documentation on transactions and retryWrites option curl -s 'https://mongodb.github.io/node-mongodb-native/5.0/interfaces/TransactionOptions.html' | grep -A 5 retryWritesLength of output: 21339
Verified: Transaction retry behavior is correctly disabled
The code now explicitly passes
retryWrites: falsetosession.withTransaction, ensuring that automatic retries are disabled. This aligns with the PR’s objective and is consistent with the MongoDB documentation, which indicates that the default behavior is to retry writes unless this option is overridden.
- Location:
packages/plugin-mongo/src/utils.ts(Lines 15-22)- Rationale: Disabling automatic transaction retries prevents potential unexpected behavior when using custom middleware.
examples/cli-mongodb/package.json (1)
1-26: New MongoDB example configuration looks goodThe package.json for the new MongoDB CLI example is well-structured with appropriate dependencies. This example will be useful for demonstrating the fixes implemented in this PR.
A few observations:
- Using MongoDB v6.12.0, which is recent and compatible with the plugin changes
- All Apibara dependencies use workspace references for local development
- Includes proper linting configuration with Biome
jaipaljadeja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks
The MongoDB had the following issues:
createCollectionon an already existing collection returns an error.withTransactiondefaults to retrying by invoking the callback multiple times, but this is unacceptable for the middleware.This PR fixes both issues.