Handle non-entity table loads in TestIndexer - #1324
Merged
Merged
Conversation
…ed-effect load createTestIndexer().process() throws `TypeError: Cannot read properties of undefined (reading 'table')` from TestIndexer.handleLoad when a cached effect's cache is loaded back in a later batch. handleLoad looks up `state.entityConfigs[tableName]` and unconditionally reads `.table`. Every entity load passes the entity's own table name (always present in entityConfigs), so the "read an entity back" framing in the report is a red herring. The one loadOrThrow whose table is not an entity is the cached-effect load (LoadLayer.loadByEffect -> effect.storageMeta.table, table name `envio_effect_<EffectName>`); that name is absent from entityConfigs, so the lookup is undefined and `.table` throws. The load only fires once the effect cache has been committed by an earlier batch and a later batch requests a not-yet-in-memory cache key, i.e. a multi-batch run with a cached effect. Single-batch simulate runs never hit it, matching the report. The repro sets full_batch_size: 1 so a two-block simulate splits into two batches the way a real multi-block indexer does. https://claude.ai/code/session_01VifhCAyRnCa5Z2CW2T3rri
…uite Keeps the cross-batch cached-effect load case but drops the repro-specific naming so future TestIndexer cases can live in the same file. https://claude.ai/code/session_01VifhCAyRnCa5Z2CW2T3rri
The earlier reproduction forced multiple batches with full_batch_size: 1. Verified against the real reporter repo (enviodev/safe-analysis-indexer, envio 3.2.0, no batch-size config): the crash reproduces naturally. Tracing it there showed the failing SafeSetup→ProxyCreation case commits the first block as its own batch (WRITEBATCH [50]) and then crashes loading `envio_effect_getSafeCreatorViaTraceTransaction` in the second batch — i.e. the split comes from ProxyCreation registering a dynamic contract one block after a prior event, not from any batch-size setting. Drop full_batch_size: 1 and reproduce the split the same way: a plain event at block 2 caches the effect (batch 1 commits), then a contractRegister event at block 3 runs in the split-off batch and re-calls the cached effect with a fresh key, whose preload loads the effect-cache table and hits the missing entityConfig. https://claude.ai/code/session_01VifhCAyRnCa5Z2CW2T3rri
handleLoad assumed every load targets a registered entity and dereferenced state.entityConfigs[tableName].table. The cached-effect load passes the effect-cache table (envio_effect_<name>), which isn't an entity, so the lookup was undefined and .table threw "Cannot read properties of undefined (reading 'table')". TestIndexer never persists effect caches, so return an empty result for non-entity tables and let the effect recompute. Move the reproduction into EventHandler.test.ts (the existing TestIndexer test file) so all TestIndexer cases live in one place, and compact its comment. https://claude.ai/code/session_01VifhCAyRnCa5Z2CW2T3rri
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesCached effect load safety fix and regression test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix a crash in TestIndexer when loading cached effect tables during batch splits. The
handleLoadfunction assumed all loaded tables were entity tables with corresponding configs, but effect cache tables (namedenvio_effect_<name>) are never persisted by TestIndexer and lack configs.Changes
handleLoadlogic in a switch that checks if the table has an entity config. Return an empty result for non-entity tables (effect caches) instead of crashing when trying to access the missing config.registerAndCachedEffectthat reproduces the cross-batch cached-effect load scenario by registering a dynamic contract at a later block, which splits processing into two batches.Implementation Details
When a dynamic contract is registered at a later block than a prior event, processing splits into two batches. The second batch's preload attempts to load the effect cache from storage. TestIndexer now gracefully handles this by returning an empty result, allowing the effect to recompute rather than crashing on the missing config.
https://claude.ai/code/session_01VifhCAyRnCa5Z2CW2T3rri
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests