Skip to content

fix(projections): harden js state handling#423

Merged
yordis merged 1 commit into
masterfrom
yordis/fix-jint-state-handler
Jul 6, 2026
Merged

fix(projections): harden js state handling#423
yordis merged 1 commit into
masterfrom
yordis/fix-jint-state-handler

Conversation

@yordis

@yordis yordis commented Jul 6, 2026

Copy link
Copy Markdown
Member
  • 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>
@cursor

cursor Bot commented Jul 6, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes projection state persistence and default event passthrough semantics; fixes correctness bugs but could alter behavior for projections that relied on the old string or body handling.

Overview
Hardens JavaScript projection state and event handling so persisted state stays valid JSON and primitive event payloads no longer break processing.

Persisted state no longer takes a shortcut that wrote raw AsString() for string-valued JS state (including bi-state slot [0]). All outputs go through the JSON serializer, so a handler returning 'hello' is stored as "hello" and can be reloaded via Load.

Event body is exposed as whatever JSON parses to (numbers, strings, booleans, null), not only objects. The no-handler fallback for JSON events uses the parsed body instead of raw data. Serialization maps non-finite numbers (NaN, ±Infinity) to JSON null instead of failing at write time.

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.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Changes 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.

Changes

Jint projection state serialization and event body fixes

Layer / File(s) Summary
Non-finite number serialization
src/EventStore.Projections.Core/Services/Interpreted/JintProjectionStateHandler.cs, src/EventStore.Projections.Core.Tests/Services/Jint/Serialization/when_serializing_state.cs
Numbers are written as JSON only if finite; NaN/Infinity/-Infinity serialize to null, verified by a new parameterized test.
Event body access and envelope handling
src/EventStore.Projections.Core/Services/Interpreted/JintProjectionStateHandler.cs, src/EventStore.Projections.Core.Tests/Services/Jint/when_accessing_event_body_with_non_object_data.cs
EventEnvelope.Body/EnsureBody now accept and return non-object JsValues (including null), and InterpreterRuntime.Handle picks Body or BodyRaw based on whether the event is JSON, validated with a new test fixture for primitive event bodies.
String state output conversion
src/EventStore.Projections.Core/Services/Interpreted/JintProjectionStateHandler.cs, src/EventStore.Projections.Core.Tests/Services/Jint/when_round_tripping_js_projection_with_string_state.cs
PrepareOutput no longer special-cases raw string state for bi-state or non-bi-state paths, always using ConvertToStringHandlingNulls, validated by new round-trip tests asserting JSON-encoded string output.

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
Loading

Poem

A rabbit hops through numbers strange,
NaN and Infinity now rearrange,
Bodies primitive, no longer barred,
Strings wrapped tight, converted hard,
Tests all green, my burrow's proud! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: hardening JavaScript projection state handling.
Description check ✅ Passed The description matches the implemented fixes for primitive event bodies, string state persistence, and non-finite number serialization.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yordis/fix-jint-state-handler

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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 win

Consider 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-Load state 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

📥 Commits

Reviewing files that changed from the base of the PR and between 35e09cd and 39c4f57.

📒 Files selected for processing (4)
  • src/EventStore.Projections.Core.Tests/Services/Jint/Serialization/when_serializing_state.cs
  • src/EventStore.Projections.Core.Tests/Services/Jint/when_accessing_event_body_with_non_object_data.cs
  • src/EventStore.Projections.Core.Tests/Services/Jint/when_round_tripping_js_projection_with_string_state.cs
  • src/EventStore.Projections.Core/Services/Interpreted/JintProjectionStateHandler.cs

@yordis yordis merged commit fefe464 into master Jul 6, 2026
21 checks passed
@yordis yordis deleted the yordis/fix-jint-state-handler branch July 6, 2026 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant