Skip to content

Conversation

@habdelra
Copy link
Contributor

Issue: recovered instances (instances that were previously an error, but then repaired) kept showing errors because error_doc was stored as jsonb null (not SQL NULL), so the index still treated them as errored even after dependencies were fixed.

the fix was to store SQL NULL for cleared JSON fields so recovered instances/modules no longer retain error state (expression.ts). Added regression coverage to confirm error_doc is SQL NULL after a recovery path (indexing-test.ts).

@habdelra habdelra requested a review from Copilot January 16, 2026 22:36
@habdelra habdelra requested a review from a team January 16, 2026 22:36
@habdelra habdelra changed the title Fix invalidation bug where string "null" was being conflated with SQL NULL Fix invalidation bug where jsonb "null" was being conflated with SQL NULL Jan 16, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a bug where recovered instances (instances that were previously in an error state but then repaired) continued to show errors because error_doc was stored as JSONB null instead of SQL NULL. The fix ensures that cleared JSON fields are stored as SQL NULL.

Changes:

  • Modified asExpressions function to distinguish between SQL NULL and JSONB null for JSON fields
  • Added regression test to verify error_doc is SQL NULL after instance recovery

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/runtime-common/expression.ts Updated asExpressions to return SQL NULL instead of JSONB null when JSON field values are null/undefined
packages/realm-server/tests/indexing-test.ts Added test assertion to verify error_doc IS NULL after instance recovery

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@backspace
Copy link
Contributor

hmm the modified test has a failed assertion 😞

@github-actions
Copy link

github-actions bot commented Jan 16, 2026

Host Test Results

    1 files  ±    0      1 suites  ±0   3h 12m 53s ⏱️ + 1h 40m 16s
1 888 tests +  206  1 872 ✅ +  207  15 💤 ± 0  1 ❌ ±0 
3 592 runs  +1 896  3 561 ✅ +1 883  30 💤 +15  1 ❌  - 1 

For more details on these failures, see this check.

Results for commit a555784. ± Comparison against base commit 4eaa075.

This pull request removes 1 and adds 207 tests. Note that renamed tests count towards both.
Chrome ‑ Global error: Uncaught TypeError: Failed to fetch at http://localhost:7357/assets/chunk.a5ff887984183a9ed614.js, line 150984  While executing test: Integration | card-copy: can copy a card 
Chrome ‑ Global error: Uncaught TypeError: Failed to fetch at http://localhost:7357/assets/chunk.84fb7a6dd0c0cf79d3c6.js, line 150984  While executing test: Integration | card-copy: copy button appears when right and left stacks are index cards and there are selections on left side 
Chrome ‑ Integration | Store: added instance that was previously not saved will begin to auto save after being added
Chrome ‑ Integration | Store: an instance can be restored after a loader reset
Chrome ‑ Integration | Store: an instance can debounce auto saves
Chrome ‑ Integration | Store: an instance can live update thru an error state
Chrome ‑ Integration | Store: an instance live updates from indexing events for a code update
Chrome ‑ Integration | Store: an instance live updates from indexing events for an instance update
Chrome ‑ Integration | Store: an instance that started out with a local ID can be restored after a loader reset
Chrome ‑ Integration | Store: an instance will NOT auto save when its data changes, if the user does not have write permissions
Chrome ‑ Integration | Store: an instance will auto save when its data changes
…

♻️ This comment has been updated with latest results.

@habdelra
Copy link
Contributor Author

hmm the modified test has a failed assertion 😞

fixed now

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.

3 participants