-
Notifications
You must be signed in to change notification settings - Fork 11
indexer: add plugins:init hook and hooks support in vcr and useTestDrizzleStorage()
#191
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
indexer: add plugins:init hook and hooks support in vcr and useTestDrizzleStorage()
#191
Conversation
📝 WalkthroughWalkthroughIntroduces a new plugins:init lifecycle hook, updates plugins to initialize on it, and modifies testing VCR to accept range and runtime hooks. Adds useTestDrizzleStorage for test seeding. Example indexer and tests updated to use new hooks and range shape. Adds prerelease change manifests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test Runner
participant VCR as VCR.run(config)
participant Indexer as Indexer
participant Hooks as indexer.hooks
participant Plugins as Plugins (drizzle/mongo/sqlite/logger)
participant Stream as Stream/Middleware
Test->>VCR: run(cassette, indexerConfig, {range, hooks})
VCR->>VCR: Merge runtime hooks with indexerConfig.hooks
VCR->>Indexer: run()
Indexer->>Hooks: callHook("plugins:init")
Hooks->>Plugins: plugins:init (initialize storage, logger, etc.)
Note over Plugins: Initialization moved from run:before to plugins:init
Indexer->>Indexer: registerMiddleware()
alt Optional before-run hook
Hooks->>Hooks: run:before
Note over Hooks: e.g., seed test DB via useTestDrizzleStorage
end
Indexer->>Stream: start processing blocks(range)
sequenceDiagram
autonumber
participant Test as Test (ethereum.test)
participant VCR as VCR.run
participant Drizzle as useTestDrizzleStorage
participant DB as Test DB
Test->>VCR: run({ range, hooks: {"run:before": fn} })
VCR->>VCR: Merge hooks
VCR->>Test: Execute hook "run:before"
Test->>Drizzle: useTestDrizzleStorage()
Drizzle->>DB: insert seed row
VCR->>VCR: Continue cassette replay/record
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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. Comment |
fracek
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.
Looks good!
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/indexer/src/indexer.ts (1)
41-70: Allow async handlers in hook signatures (especially for plugins:init).Many init tasks (DB seeding, connections) are async. Typing hooks as MaybePromise improves DX and matches call sites that await callHook.
-export interface IndexerHooks<TFilter, TBlock> { - "plugins:init": () => void; - "run:before": () => void; - "run:after": () => void; - "connect:before": ({ +export interface IndexerHooks<TFilter, TBlock> { + "plugins:init": () => void | Promise<void>; + "run:before": () => void | Promise<void>; + "run:after": () => void | Promise<void>; + "connect:before": ({ request, options, }: { request: StreamDataRequest<TFilter>; options: StreamDataOptions; - }) => void; + }) => void | Promise<void>; "connect:after": ({ request, }: { request: StreamDataRequest<TFilter>; - }) => void; + }) => void | Promise<void>; "connect:factory": ({ request, endCursor, }: { request: StreamDataRequest<TFilter>; endCursor?: Cursor; - }) => void; - "handler:middleware": ({ use }: { use: UseMiddlewareFunction }) => void; - message: ({ message }: { message: StreamDataResponse<TBlock> }) => void; - "message:invalidate": ({ message }: { message: Invalidate }) => void; - "message:finalize": ({ message }: { message: Finalize }) => void; - "message:heartbeat": () => void; - "message:systemMessage": ({ message }: { message: SystemMessage }) => void; + }) => void | Promise<void>; + "handler:middleware": ({ + use, + }: { + use: UseMiddlewareFunction; + }) => void | Promise<void>; + message: ({ + message, + }: { + message: StreamDataResponse<TBlock>; + }) => void | Promise<void>; + "message:invalidate": ({ message }: { message: Invalidate }) => void | Promise<void>; + "message:finalize": ({ message }: { message: Finalize }) => void | Promise<void>; + "message:heartbeat": () => void | Promise<void>; + "message:systemMessage": ({ message }: { message: SystemMessage }) => void | Promise<void>; }If you prefer a helper alias, add near the top:
type MaybePromise<T> = T | Promise<T>;and then use MaybePromise for each hook.
packages/plugin-drizzle/src/index.ts (1)
211-277: Init migration/backfill improvements; guard test-DB exposure; improve backoff; avoid ts-ignore
- Only expose the raw DB in explicit test mode.
- Use exponential backoff and make the error message reflect MAX_RETRIES.
- Replace the ts-ignore on migrate with proper typing or a helper overload.
const internalContext = useInternalContext(); const context = useIndexerContext(); const logger = useLogger(); - // For testing purposes using vcr. - context[DRIZZLE_STORAGE_DB_PROPERTY] = db; + // Make test DB accessible only when explicitly enabled. + if (process.env["APIBARA_EXPOSE_TEST_DB"] === "true") { + context[DRIZZLE_STORAGE_DB_PROPERTY] = db; + } @@ - if (migrateOptions && !migrationsApplied) { - // @ts-ignore type mismatch for db - await migrate(db, migrateOptions); + if (migrateOptions && !migrationsApplied) { + await migrate(db as unknown as Parameters<typeof migrate>[0], migrateOptions); migrationsApplied = true; logger.success("Migrations applied"); } @@ - if (retries === MAX_RETRIES) { + if (retries === MAX_RETRIES) { if (error instanceof DrizzleStorageError) { throw error; } throw new DrizzleStorageError( - "Initialization failed after 5 retries", + `Initialization failed after ${MAX_RETRIES} retries`, { cause: error, }, ); } - await sleep(retries * 1000); + const delay = (1000 * 2 ** retries) + Math.floor(Math.random() * 250); + await sleep(delay); retries++;If you prefer an option instead of env, we can add
exposeTestDb?: booleanto DrizzleStorageOptions and guard on it.
🧹 Nitpick comments (11)
change/@apibara-plugin-drizzle-ea1d203a-a6e4-4043-be14-c3e737c25be5.json (1)
3-3: Normalize hook name in release note to match code ("plugins:init").Use the pluralized hook id for consistency with IndexerHooks and code.
- "comment": "plugin-drizzle: add useTestDrizzleStorage and use `plugin:init` hook", + "comment": "plugin-drizzle: add useTestDrizzleStorage and use `plugins:init` hook",packages/indexer/src/plugins/context.ts (1)
6-9: Optionally narrow types and harden context writes.Consider making values a Partial and freezing the merged object to catch shape mistakes at runtime.
-export function internalContext<TFilter, TBlock, TTxnParams>( - values: Record<string, unknown>, -) { +export function internalContext<TFilter, TBlock, TTxnParams>( + values: Partial<InternalContext>, +) {And inside the hook:
- ctx[INTERNAL_CONTEXT_PROPERTY] = { + ctx[INTERNAL_CONTEXT_PROPERTY] = Object.freeze({ ...(ctx[INTERNAL_CONTEXT_PROPERTY] || {}), ...values, - }; + });Also applies to: 26-40
change/@apibara-plugin-mongo-32bec2ae-dcc5-4d53-a76a-78f3d7340a25.json (1)
3-3: Fix hook name consistency (“plugins:init”).Align the note with the actual hook id used in code.
- "comment": "plugin-mongo: use `plugin:init` hook", + "comment": "plugin-mongo: use `plugins:init` hook",packages/indexer/src/plugins/logger.ts (1)
16-28: Init on plugins:init looks right; minor API naming nit.Parameter is a single reporter; consider renaming to reporter to avoid confusion with a logger instance.
-export function logger<TFilter, TBlock, TTxnParams>({ - logger, -}: { logger?: ConsolaReporter } = {}) { +export function logger<TFilter, TBlock, TTxnParams>({ + reporter, +}: { reporter?: ConsolaReporter } = {}) { return defineIndexerPlugin<TFilter, TBlock>((indexer) => { indexer.hooks.hook("plugins:init", () => { const ctx = useIndexerContext(); - if (logger) { - ctx.logger = consola.create({ reporters: [logger] }); + if (reporter) { + ctx.logger = consola.create({ reporters: [reporter] }); } else { ctx.logger = consola.create({}); }packages/indexer/src/indexer.ts (1)
475-476: Confirm intended semantics of run:after (fires per message).Currently invoked once per processed message, not just at stream end. If that’s intentional, consider documenting it to avoid surprise.
change/@apibara-plugin-sqlite-39d33299-a591-4c41-8661-da9efd0f214d.json (1)
3-3: Unify to “plugins:init” in the comment.Match the actual hook id used in code.
- "comment": "plugin-sqlite: use `plugin:init` hook", + "comment": "plugin-sqlite: use `plugins:init` hook",examples/cli-drizzle/indexers/1-evm.indexer.ts (1)
46-50: Prefer logger over console in hooksUse the indexer logger for consistency and to respect configured log levels.
hooks: { - "run:before": () => { - console.log("run:before hook called from indexer"); - }, + "run:before": () => { + const logger = useLogger(); + logger.info("run:before hook called from indexer"); + }, },packages/plugin-sqlite/src/index.ts (1)
84-137: Init moved to plugins:init looks right; tighten retry/backoff and logsGood migration. Two small robustness nits:
- Make the error message reflect MAX_RETRIES.
- Use exponential backoff (with jitter) instead of a 0 ms first sleep.
- const { indexerName: indexerFileName, availableIndexers } = - useInternalContext(); + const { indexerName: indexerFileName } = useInternalContext();- throw new SqliteStorageError( - "Initialization failed after 5 retries", + throw new SqliteStorageError( + `Initialization failed after ${MAX_RETRIES} retries`, { cause: error, }, );- await sleep(retries * 1000); + // Exponential backoff with basic jitter + const delay = (1000 * 2 ** retries) + Math.floor(Math.random() * 250); + await sleep(delay); retries++;packages/plugin-mongo/src/index.ts (1)
68-94: Init on plugins:init is correct; consider adding retry/backoff for transient failuresUnlike sqlite/drizzle, Mongo init does not retry on transient startup/connectivity errors. Wrapping the transactional setup with a small retry loop (exponential backoff) will reduce test flakes and cold-start issues.
If you want, I can provide a minimal retry wrapper consistent with other plugins.
examples/cli-drizzle/test/ethereum.test.ts (1)
19-33: Nice use of run hooks; make seeding idempotent and drop noisy console.logAvoid flakiness on reruns by making the seed insert idempotent; prefer test logger or no logs.
hooks: { "run:before": async () => { - console.log("run:before hook called from test function"); const db = useTestDrizzleStorage(); - await db.insert(ethereumUsdcTransfers).values({ + await db.insert(ethereumUsdcTransfers).values({ number: 1, hash: "0x0000000000000000000000000000000000000000000000000000000000000000", - }); + }).onConflictDoNothing(); }, },packages/indexer/src/testing/index.ts (1)
26-29: Consider making config optional for backward compatibilityAllow calling run(...) with only the old range param by providing defaults, easing upgrades for existing tests.
- config: { + config: { range: { fromBlock: bigint; toBlock: bigint }; hooks?: NestedHooks<IndexerHooks<TFilter, TBlock>>; - }, + } = { range: { fromBlock: 0n, toBlock: 0n } },(Adjust defaults to your preferred sentinel or overload the signature.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
change/@apibara-indexer-9fa2931c-4a7e-4fb8-9f8b-cf50b02bf94f.json(1 hunks)change/@apibara-plugin-drizzle-ea1d203a-a6e4-4043-be14-c3e737c25be5.json(1 hunks)change/@apibara-plugin-mongo-32bec2ae-dcc5-4d53-a76a-78f3d7340a25.json(1 hunks)change/@apibara-plugin-sqlite-39d33299-a591-4c41-8661-da9efd0f214d.json(1 hunks)examples/cli-drizzle/indexers/1-evm.indexer.ts(1 hunks)examples/cli-drizzle/test/ethereum.test.ts(3 hunks)examples/cli-drizzle/test/starknet.test.ts(1 hunks)packages/indexer/src/indexer.ts(2 hunks)packages/indexer/src/plugins/context.ts(1 hunks)packages/indexer/src/plugins/logger.ts(1 hunks)packages/indexer/src/testing/index.ts(2 hunks)packages/plugin-drizzle/src/index.ts(2 hunks)packages/plugin-mongo/src/index.ts(1 hunks)packages/plugin-sqlite/src/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/plugin-drizzle/src/index.ts (2)
packages/plugin-drizzle/src/constants.ts (1)
DRIZZLE_STORAGE_DB_PROPERTY(2-2)packages/plugin-drizzle/src/utils.ts (1)
DrizzleStorageError(11-16)
packages/indexer/src/testing/index.ts (2)
packages/indexer/src/indexer.ts (1)
IndexerHooks(41-70)packages/indexer/src/vcr/config.ts (2)
VcrConfig(3-5)CassetteOptions(7-11)
examples/cli-drizzle/test/ethereum.test.ts (2)
packages/plugin-drizzle/src/index.ts (1)
useTestDrizzleStorage(77-92)examples/cli-drizzle/lib/schema.ts (1)
ethereumUsdcTransfers(9-13)
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (8)
packages/indexer/src/plugins/context.ts (1)
10-22: Good move to plugins:init; aligns init timing with context availability.This ensures ctx.debug is set before plugin init and that the async context exists. Looks correct.
change/@apibara-indexer-9fa2931c-4a7e-4fb8-9f8b-cf50b02bf94f.json (1)
1-7: LGTM — matches implemented behavior.Comment accurately references “plugins:init” and VCR hooks support.
examples/cli-drizzle/test/starknet.test.ts (1)
18-20: Range shape update looks good.The new { range } API is used correctly with bigint bounds.
packages/indexer/src/indexer.ts (1)
234-236: Correct placement: initialize plugins before middleware registration.This ordering ensures plugins can populate context and register resources before middleware is composed. Nice.
examples/cli-drizzle/test/ethereum.test.ts (2)
5-5: LGTM: targeted import for test-only storageImporting useTestDrizzleStorage in tests aligns with the intended usage. Just ensure it stays out of production paths.
Use the repo scan in my other comment to keep it test-only.
41-44: Snapshot change acknowledgedSeed row at the top is expected due to the run:before insert.
packages/indexer/src/testing/index.ts (2)
35-39: Confirm hook merge precedencemergeHooks(indexerConfig.hooks, config.hooks) appends user-supplied hooks; confirm this precedence is what you want when both provide the same lifecycle (e.g., connect:before). If not, swap arguments.
You can validate with a small unit test that both handlers fire in the intended order.
43-47: Range mapping looks correctfromBlock/toBlock → starting/ending cursor wiring is consistent with VCR.
resolves #190