fix(projections): harden js state handling#423
Conversation
yordis
commented
Jul 6, 2026
- JavaScript projections should not fault when valid JSON event bodies are primitives rather than objects.
- Persisted projection state should remain valid JSON so string states survive restart and reload paths.
- Non-finite JavaScript numbers should serialize predictably instead of surfacing as runtime projection failures.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryMedium Risk Overview Persisted state no longer takes a shortcut that wrote raw Event New Jint tests cover primitive bodies, string-state round-trip (normal and bi-state), and non-finite number serialization. Reviewed by Cursor Bugbot for commit 39c4f57. Bugbot is set up for automated code reviews on this repo. Configure here. |
WalkthroughChanges modify JintProjectionStateHandler to serialize non-finite JS numbers as null, adjust EventEnvelope body access/creation to handle non-object and null values, select event body by JSON type when no handler matches, and consistently convert output state via ConvertToStringHandlingNulls. New NUnit tests cover these behaviors. ChangesJint projection state serialization and event body fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Event as Event Envelope
participant Runtime as InterpreterRuntime.Handle
participant Handler as JintProjectionStateHandler
participant Output as PrepareOutput
Event->>Runtime: dispatch event (IsJson flag)
alt no event-specific or $any handler
Runtime->>Event: select Body (if JSON) or BodyRaw
Event-->>Runtime: JsValue (object, primitive, or null)
end
Runtime->>Handler: compute newState from JsValue
Handler->>Output: PrepareOutput(state)
Output->>Output: ConvertToStringHandlingNulls(state)
alt value is finite number
Output-->>Handler: JSON number
else non-finite number
Output-->>Handler: null
end
Handler-->>Runtime: serialized state string
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/EventStore.Projections.Core.Tests/Services/Jint/when_round_tripping_js_projection_with_string_state.cs (1)
30-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider asserting reloaded state content, not just no-throw.
Assert.DoesNotThrow(() => _stateHandler.Load(state))only proves the JSON is parseable; it doesn't confirm the reloaded engine state actually equals"hello". Given the round-trip intent of these tests, asserting on the post-Loadstate value would strengthen the regression coverage.Also applies to: 61-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/EventStore.Projections.Core.Tests/Services/Jint/when_round_tripping_js_projection_with_string_state.cs` at line 30, The round-trip test only checks that _stateHandler.Load(state) does not throw, but it never verifies the loaded state value. Update the assertions in when_round_tripping_js_projection_with_string_state to assert the reloaded state content after Load on the relevant state handler/engine object, and do the same in the other matching case, so the test confirms the state still equals "hello" rather than just being parseable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/EventStore.Projections.Core.Tests/Services/Jint/when_round_tripping_js_projection_with_string_state.cs`:
- Line 30: The round-trip test only checks that _stateHandler.Load(state) does
not throw, but it never verifies the loaded state value. Update the assertions
in when_round_tripping_js_projection_with_string_state to assert the reloaded
state content after Load on the relevant state handler/engine object, and do the
same in the other matching case, so the test confirms the state still equals
"hello" rather than just being parseable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3eaf4233-ee42-4264-8717-bd6b5552056a
📒 Files selected for processing (4)
src/EventStore.Projections.Core.Tests/Services/Jint/Serialization/when_serializing_state.cssrc/EventStore.Projections.Core.Tests/Services/Jint/when_accessing_event_body_with_non_object_data.cssrc/EventStore.Projections.Core.Tests/Services/Jint/when_round_tripping_js_projection_with_string_state.cssrc/EventStore.Projections.Core/Services/Interpreted/JintProjectionStateHandler.cs