Skip to content

Block writes during schema migration to prevent conflicts#8475

Open
pubkey wants to merge 5 commits intomasterfrom
claude/prevent-migration-writes-vXhXs
Open

Block writes during schema migration to prevent conflicts#8475
pubkey wants to merge 5 commits intomasterfrom
claude/prevent-migration-writes-vXhXs

Conversation

@pubkey
Copy link
Copy Markdown
Owner

@pubkey pubkey commented May 5, 2026

This PR contains:

  • A BUGFIX
  • IMPROVED TESTS
  • IMPROVED DOCS

Describe the problem you have without this PR

When a schema migration is running on an RxCollection, external writes (insert, upsert, bulkInsert, etc.) could race with the migration's replication process. This could cause confusing RC_PUSH errors or other conflicts instead of failing fast with a clear error message.

Additionally, writes were allowed even when a migration was pending but not yet started, which could interfere with the migration process.

Solution

This PR implements a migrationInProgress flag on RxCollection that:

  1. Blocks all writes during migration: The flag is set when a migration starts and cleared when it completes, fails, or is cancelled. Any write attempt while the flag is true throws a clear COL25 error.

  2. Blocks writes when migration is pending: The flag is optimistically set when a collection is created (for schema version > 0), then cleared if no migration is actually needed. This prevents writes from interfering with pending migrations.

  3. Provides clear error messaging: Instead of racing errors, users get an immediate COL25 error with guidance to wait for the migration to complete.

Changes made:

  • Added migrationInProgress property to RxCollectionBase
  • Created isWriteAllowed() helper function that checks both closed state and migration state
  • Replaced ensureRxCollectionIsNotClosed() calls with isWriteAllowed() in all write methods (insert, bulkInsert, upsert, bulkUpsert, incrementalUpsert, bulkRemove)
  • Added migration plugin hook to set migrationInProgress flag when collection is created
  • Updated RxMigrationState.startMigration() to manage the flag throughout the migration lifecycle
  • Added comprehensive tests covering: blocking writes during migration, blocking writes when migration is pending, resetting flag on error, and racing scenarios
  • Added COL25 error definition and documentation warning

Test Plan

Added 4 new unit tests in migration-schema.test.ts:

  • Verifies writes are blocked while migration runs and allowed after completion
  • Verifies bulkInsert that races with migration start is blocked
  • Verifies writes are blocked when migration is needed but not yet started
  • Verifies migrationInProgress flag is reset on migration error

All tests pass and cover the key scenarios where writes could interfere with migrations.

https://claude.ai/code/session_01PFvmQnnz2GCozXqMfCjYvy

claude added 5 commits May 5, 2026 12:20
Outside writes that race with a running migration could conflict with the
migration replication, which surfaced as confusing RC_PUSH errors. Block
inserts/upserts/removes and RxDocument modifications via a new
ensureRxCollectionIsNotMigrating guard (COL25) while migrationInProgress
is true on the collection.
…gration is needed

- Replace ensureRxCollectionIsNotClosed + ensureRxCollectionIsNotMigrating
  pairs at write call sites with a single isWriteAllowed helper that
  asserts both conditions.
- Set migrationInProgress eagerly in the migration plugin's
  createRxCollection hook for schemas with version > 0, so writes are
  refused with COL25 even before startMigration() has been called.
  Cleared once migrationNeeded() resolves false or once the migration
  finishes/errors/cancels.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

✅ Verify Test Reproduction: Tests FAILED without the fix (expected)

This confirms the changed tests correctly reproduce the bug that the source changes fix.

This workflow runs the changed tests without the source fix to verify they reproduce the bug.

Show output
...(truncated, showing last 200 of 3518 lines)

  replication-webrtc.test.ts
    init
      �[32m✓ �[39mimport WebRTC polyfills on Node.js
    utils
      .isMasterInWebRTCReplication()
        �[32m✓ �[39mshould have exactly one master vilbijx - velfalz
        �[32m✓ �[39mshould have exactly one master fsaqhhg - punxuxu
        �[32m✓ �[39mshould have exactly one master ygihfiw - wpyrvhn
        �[32m✓ �[39mshould have exactly one master pifcpam - ajywatb
        �[32m✓ �[39mshould have exactly one master hyvxyvu - pcburlt
        �[32m✓ �[39mshould have exactly one master pshjlzm - hjxvtcc
        �[32m✓ �[39mshould have exactly one master mwocrha - ajyzoft
        �[32m✓ �[39mshould have exactly one master edspqen - kcxmwbi
        �[32m✓ �[39mshould have exactly one master sktjhaj - xpdymcj
        �[32m✓ �[39mshould have exactly one master rhubaip - nmhjjiq
    basic CRUD
      �[32m✓ �[39mshould stream changes over the replication to other collections

  encryption.test.ts
    init
      �[32m✓ �[39mcreate storage
    basics
      .encryptString()
        �[32m✓ �[39mstring
      .decryptString()
        �[32m✓ �[39mstring
        �[32m✓ �[39mshould encrypt and decrypt an extremely long string
        �[32m✓ �[39mshould encrypt and decrypt an extremely long password
    RxDatabase creation
      �[32m✓ �[39mshould crash with invalid password (empty object)
      �[32m✓ �[39mBUG: should have stored the password hash when creating the database
      �[32m✓ �[39mprevent 2 instances with different passwords on same adapter
    RxCollection creation
      �[32m✓ �[39mcreate encrypted collection
    RxCollection.insert()
      �[32m✓ �[39mshould insert one encrypted value (string)
      �[32m✓ �[39mshould insert one encrypted value (object)
      �[32m✓ �[39m#5624 insert with really big encrypted string
    RxDocument.save()
      �[32m✓ �[39mshould save one encrypted value (string)
      �[32m✓ �[39mshould save one encrypted value (object)
    replication
      �[32m✓ �[39mreplication state meta should not contain a secret in cleartext
    ISSUES
      �[32m✓ �[39m#837 Recover from wrong database password
      �[32m✓ �[39m#917 Unexpected end of JSON input
      �[32m✓ �[39mshould work with encrypted fields that have maxLength in schema
      �[32m✓ �[39mshould work with encrypted object fields that have nested properties
      �[32m✓ �[39mshould correctly encrypt and decrypt nested fields with dot-notation paths and non-string types
      �[32m✓ �[39mshould throw when encrypted contains overlapping parent and child paths
      #157 Cannot sort on field(s) "XXX" when using the default index
        �[32m✓ �[39mschema example 1
        �[32m✓ �[39mschema example 2
    SECURITY
      �[32m✓ �[39mshould not expose the database password as an enumerable property
      �[32m✓ �[39mshould not leak the password in error parameters when password is too short

  rx-state.test.ts (useSchemaValidator: true)
    helper
      .nextRxStateId()
        �[32m✓ �[39mshould increment in sort order
    creation
      �[32m✓ �[39mcalling addState twice should give the same instance
    write state data
      �[32m✓ �[39mshould write without error
      �[32m✓ �[39mwrite multiple times at once
      �[32m✓ �[39mupdate nested
      �[32m✓ �[39mupdate complete state at once
      �[32m✓ �[39mdoing many writes should end up in a single persistence write to the storage
    .get()
      �[32m✓ �[39mshould get root state
      �[32m✓ �[39mshould get the updated value
      �[32m✓ �[39mshould get nested values
      �[32m✓ �[39mshould not throw on undefined values
    .get$()
      �[32m✓ �[39mshould emit the correct data
      �[32m✓ �[39mshould emit the correct data via proxy-getter a$
    .get$$()
      �[32m✓ �[39mshould get the correct object
    cleanup
      �[32m✓ �[39mshould merge the state documents data on cleanup
    multiInstance
      �[32m✓ �[39mwrite with two states at once
      �[32m✓ �[39mwrite with two states to nested at once
      �[32m✓ �[39mget changes from other state
      �[32m✓ �[39mshould have a deterministic output when 2 instances write at the same time
      �[32m✓ �[39mshould have a deterministic output when 2 instances write to different fields
      �[32m✓ �[39mshould recover the same state from disc on the other side
      �[32m✓ �[39mshould emit the correct data for all states
    issues
      �[32m✓ �[39mRxState.property$ should be stable for initial synchronous get and subsequent subscription
      �[32m✓ �[39mset() with empty path should pass the current state to the modifier
      �[32m✓ �[39mshould recover correct state from disk after full-state replacement with set('')
      �[32m✓ �[39mbad rx-state after cleanup
      �[32m✓ �[39m$ observable should not emit duplicate values on a single write
      �[32m✓ �[39mget$() should emit the current value when subscribed, not a stale value from creation time
      �[32m✓ �[39m_cleanup() should return true to signal completion
      �[32m✓ �[39mwrite queue should recover after a modifier throws

  rx-state.test.ts (useSchemaValidator: false)
    helper
      .nextRxStateId()
        �[32m✓ �[39mshould increment in sort order
    creation
      �[32m✓ �[39mcalling addState twice should give the same instance
    write state data
      �[32m✓ �[39mshould write without error
      �[32m✓ �[39mwrite multiple times at once
      �[32m✓ �[39mupdate nested
      �[32m✓ �[39mupdate complete state at once
      �[32m✓ �[39mdoing many writes should end up in a single persistence write to the storage
    .get()
      �[32m✓ �[39mshould get root state
      �[32m✓ �[39mshould get the updated value
      �[32m✓ �[39mshould get nested values
      �[32m✓ �[39mshould not throw on undefined values
    .get$()
      �[32m✓ �[39mshould emit the correct data
      �[32m✓ �[39mshould emit the correct data via proxy-getter a$
    .get$$()
      �[32m✓ �[39mshould get the correct object
    cleanup
      �[32m✓ �[39mshould merge the state documents data on cleanup
    multiInstance
      �[32m✓ �[39mwrite with two states at once
      �[32m✓ �[39mwrite with two states to nested at once
      �[32m✓ �[39mget changes from other state
      �[32m✓ �[39mshould have a deterministic output when 2 instances write at the same time
      �[32m✓ �[39mshould have a deterministic output when 2 instances write to different fields
      �[32m✓ �[39mshould recover the same state from disc on the other side
      �[32m✓ �[39mshould emit the correct data for all states
    issues
      �[32m✓ �[39mRxState.property$ should be stable for initial synchronous get and subsequent subscription
      �[32m✓ �[39mset() with empty path should pass the current state to the modifier
      �[32m✓ �[39mshould recover correct state from disk after full-state replacement with set('')
      �[32m✓ �[39mbad rx-state after cleanup
      �[32m✓ �[39m$ observable should not emit duplicate values on a single write
      �[32m✓ �[39mget$() should emit the current value when subscribed, not a stale value from creation time
      �[32m✓ �[39m_cleanup() should return true to signal completion
      �[32m✓ �[39mwrite queue should recover after a modifier throws

  migration-schema.test.ts
    .create() with migrationStrategies
      positive
        �[32m✓ �[39mok to create with strategies
        �[32m✓ �[39mcreate same collection with different schema-versions
      negative
        �[32m✓ �[39mshould throw when array
        �[32m✓ �[39mshould throw when property no number
        �[32m✓ �[39mshould throw when property no non-float-number
        �[32m✓ �[39mshould throw when property-value no function
        �[32m✓ �[39mthrow when strategy missing
    getOldCollectionMeta()
      �[32m✓ �[39mshould NOT get an older version
      �[32m✓ �[39mshould get an older version
    migration basics
      .remove()
        �[32m✓ �[39mshould delete the old storage instance with all its content
      .migrate()
        �[32m✓ �[39mshould resolve finished when no docs
        �[32m✓ �[39mshould resolve finished when some docs are in the collection
        �[32m✓ �[39mshould emit status updates
        �[32m✓ �[39mshould remove the document when migration-strategy returns null
        �[32m✓ �[39mshould throw when document cannot be migrated
      .migratePromise()
        positive
          �[32m✓ �[39mshould resolve when nothing to migrate
          �[32m✓ �[39mshould resolve when migrating data
        negative
          �[32m✓ �[39mshould reject when migration fails
ERROR LOG: �[36m'init.test.ts: browser uncaught error:'�[39m
ERROR LOG: �[36m'Uncaught TypeError: path.dirname is not a function'�[39m
�[33m05 05 2026 14:18:36.016:WARN [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected (0 times) , because no message in 300000 ms.
�[31mChrome Headless 147.0.0.0 (Linux 0.0.0) ERROR�[39m
  Disconnected , because no message in 300000 ms.
�[32m05 05 2026 14:18:36.018:INFO [karma-server]: �[39mRestarting Chrome Headless 147.0.0.0 (Linux 0.0.0) (1 of 4 attempts)
�[32m05 05 2026 14:18:36.373:INFO [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected browser returned on socket GwLI1dVhfqMQgJkSAAAD with id 458074.
�[33m05 05 2026 14:23:36.374:WARN [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected (1 times) , because no message in 300000 ms.
�[31mChrome Headless 147.0.0.0 (Linux 0.0.0) ERROR�[39m
  Disconnected , because no message in 300000 ms.
�[32m05 05 2026 14:23:36.374:INFO [karma-server]: �[39mRestarting Chrome Headless 147.0.0.0 (Linux 0.0.0) (2 of 4 attempts)
�[32m05 05 2026 14:23:36.691:INFO [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected browser returned on socket ATfcNclQ5Yy7ewd7AAAF with id 458074.
�[33m05 05 2026 14:28:36.691:WARN [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected (2 times) , because no message in 300000 ms.
�[31mChrome Headless 147.0.0.0 (Linux 0.0.0) ERROR�[39m
  Disconnected , because no message in 300000 ms.
�[32m05 05 2026 14:28:36.691:INFO [karma-server]: �[39mRestarting Chrome Headless 147.0.0.0 (Linux 0.0.0) (3 of 4 attempts)
�[32m05 05 2026 14:28:37.029:INFO [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected browser returned on socket CZCz8dT8lDIRHaeOAAAH with id 458074.
�[33m05 05 2026 14:33:37.029:WARN [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected (3 times) , because no message in 300000 ms.
�[31mChrome Headless 147.0.0.0 (Linux 0.0.0) ERROR�[39m
  Disconnected , because no message in 300000 ms.
�[32m05 05 2026 14:33:37.029:INFO [karma-server]: �[39mRestarting Chrome Headless 147.0.0.0 (Linux 0.0.0) (4 of 4 attempts)
�[32m05 05 2026 14:33:37.344:INFO [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected browser returned on socket 2wcwls5mN89gGr_wAAAJ with id 458074.
�[33m05 05 2026 14:38:37.344:WARN [Chrome Headless 147.0.0.0 (Linux 0.0.0)]: �[39mDisconnected (4 times) , because no message in 300000 ms.
�[31mChrome Headless 147.0.0.0 (Linux 0.0.0) ERROR�[39m
  Disconnected , because no message in 300000 ms.

Chrome Headless 147.0.0.0 (Linux 0.0.0): Executed 1097 of 1294�[31m DISCONNECTED�[39m (26 mins 5.435 secs / 43.545 secs)



View full workflow run

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.

2 participants