Fix: roll back meta document and storage instance when createRxCollection() fails in addCollections()

orga/changelog/fix-failed-collection-creation-leaves-meta-document.json

@@ -0,0 +1,6 @@
+{
+    "$schema": "./schema.json",
+    "text": "FIX `addCollections()` left the collection meta document and the storage instance behind when the creation of the collection failed after the storage instance was created (for example with `COL17`), so that adding the collection again with an adjusted schema threw `DB6`",
+    "links": [],
+    "priority": 4
+}

src/rx-database.ts

@@ -54,6 +54,7 @@ import {
 } from './rx-error.ts';
 import {
     createRxSchema,
+    getPreviousVersions,
     RxSchema
 } from './rx-schema.ts';
 import {
@@ -415,6 +416,7 @@ export class RxDatabaseBase<
          * so we can overlap these I/O operations to reduce time-to-first-insert.
          */
         const collectionStorageInstancePromises: { [key: string]: Promise<RxStorageInstance<any, any, any>>; } = {};
+        const storageInstanceCreationParamsByCollectionName: { [key: string]: RxStorageInstanceCreationParams<any, any>; } = {};
         Object.keys(collectionCreators).forEach((collectionName) => {
             const useArgs = useArgsByCollectionName[collectionName];
             const storageInstanceCreationParams: RxStorageInstanceCreationParams<any, any> = {
@@ -428,6 +430,7 @@ export class RxDatabaseBase<
                 devMode: overwritable.isDevMode()
             };
             runPluginHooks('preCreateRxStorageInstance', storageInstanceCreationParams);
+            storageInstanceCreationParamsByCollectionName[collectionName] = storageInstanceCreationParams;
             const promise = createRxCollectionStorageInstance(
                 this.asRxDatabase,
                 storageInstanceCreationParams
@@ -505,6 +508,7 @@ export class RxDatabaseBase<
             throw err;
         }
 
+        const conflictingMetaDocIds = new Set(putDocsResult.error.map(error => error.documentId));
         const ret: { [key in keyof CreatedCollections]: RxCollection<any, {}, {}, {}, Reactivity> } = {} as any;
         await Promise.all(
             Object.keys(collectionCreators).map(async (collectionName) => {
@@ -513,6 +517,44 @@ export class RxDatabaseBase<
                 const collection = await createRxCollection({
                     ...useArgs,
                     storageInstance
+                }).catch(async (err) => {
+                    /**
+                     * If the creation fails, the meta document and the storage instance
+                     * that have been created by this call must not stay,
+                     * so that the collection can be added again with another schema.
+                     * When a meta document of a previous schema version exists,
+                     * a migration might have written documents into the new storage instance.
+                     * Then everything is kept so that the migration can be continued later.
+                     * A database that is closing or closed keeps its data,
+                     * and an error of the rollback must not hide the creation error.
+                     */
+                    if (this.closePromise) {
+                        throw err;
+                    }
+                    const jsonSchema = useArgs.schema.jsonSchema;
+                    try {
+                        const previousMetaDocs = await this.internalStore.findDocumentsById(
+                            getPreviousVersions(jsonSchema).map(version => getPrimaryKeyOfInternalDocument(
+                                collectionName + '-' + version,
+                                INTERNAL_CONTEXT_COLLECTION
+                            )),
+                            false
+                        );
+                        if (
+                            !conflictingMetaDocIds.has(getPrimaryKeyOfInternalDocument(
+                                _collectionNamePrimary(collectionName, jsonSchema),
+                                INTERNAL_CONTEXT_COLLECTION
+                            )) &&
+                            previousMetaDocs.length === 0
+                        ) {
+                            const newStorageInstance = await this.storage.createStorageInstance(
+                                storageInstanceCreationParamsByCollectionName[collectionName]
+                            );
+                            await newStorageInstance.remove();
+                            await this.removeCollectionDoc(collectionName, jsonSchema);
+                        }
+                    } catch { }
+                    throw err;
                 });
                 (ret as any)[collectionName] = collection;
 

test/unit/migration-schema.test.ts

@@ -53,6 +53,12 @@ import { ensureReplicationHasNoErrors } from '../../plugins/test-utils/index.mjs
 import { SimpleHumanAgeDocumentType } from '../../src/plugins/test-utils/schema-objects.ts';
 import { RxDBLeaderElectionPlugin } from '../../plugins/leader-election/index.mjs';
 import { RxDBUpdatePlugin } from '../../plugins/update/index.mjs';
+import { getRxStorageDexie } from '../../plugins/storage-dexie/index.mjs';
+import { wrappedValidateAjvStorage } from '../../plugins/validate-ajv/index.mjs';
+import {
+    indexedDB as fakeIndexedDB,
+    IDBKeyRange as fakeIDBKeyRange
+} from 'fake-indexeddb';
 
 
 describe('migration-schema.test.ts', function () {
@@ -737,6 +743,90 @@ describe('migration-schema.test.ts', function () {
 
                 db2.close();
             });
+            it('should not reject with the error of the closed storage when the database is closed during a failing auto migration', async () => {
+                /**
+                 * Uses the dexie storage because its closed instances reject all operations.
+                 */
+                const name = randomToken(10);
+                const getStorage = () => wrappedValidateAjvStorage({
+                    storage: getRxStorageDexie({
+                        indexedDB: fakeIndexedDB,
+                        IDBKeyRange: fakeIDBKeyRange
+                    })
+                });
+                const db = await createRxDatabase({
+                    name,
+                    storage: getStorage(),
+                    multiInstance: false
+                });
+                await db.addCollections({
+                    human: {
+                        schema: schemas.simpleHuman
+                    }
+                });
+                await db.human.insert(schemaObjects.simpleHumanAge({ passportId: 'local-1' }));
+                await db.close();
+
+                const db2 = await createRxDatabase({
+                    name,
+                    storage: getStorage(),
+                    multiInstance: false
+                });
+                let strategyStarted: () => void = () => { };
+                const strategyStartedPromise = new Promise<void>(res => strategyStarted = res);
+                let releaseStrategy: () => void = () => { };
+                const releaseStrategyPromise = new Promise<void>(res => releaseStrategy = res);
+                const addResultPromise = db2.addCollections({
+                    human: {
+                        schema: schemas.simpleHumanV3,
+                        autoMigrate: true,
+                        migrationStrategies: {
+                            1: d => d,
+                            2: d => d,
+                            3: async () => {
+                                strategyStarted();
+                                await releaseStrategyPromise;
+                                throw new TypeError('strategy failed');
+                            }
+                        }
+                    }
+                }).then(
+                    () => undefined,
+                    (err: Error) => err
+                );
+                await strategyStartedPromise;
+                await db2.close();
+                releaseStrategy();
+
+                const error = ensureNotFalsy(await addResultPromise);
+                assert.ok(
+                    !error.message.includes('is closed'),
+                    'must not be the error of the closed storage: ' + error.message
+                );
+
+                const db3 = await createRxDatabase({
+                    name,
+                    storage: getStorage(),
+                    multiInstance: false
+                });
+                const cols3 = await db3.addCollections({
+                    human: {
+                        schema: schemas.simpleHumanV3,
+                        autoMigrate: true,
+                        migrationStrategies: {
+                            1: d => d,
+                            2: d => d,
+                            3: d => {
+                                d.age = parseInt(d.age, 10);
+                                return d;
+                            }
+                        }
+                    }
+                });
+                const docs = await cols3.human.find().exec();
+                assert.strictEqual(docs.length, 1);
+                await db3.remove();
+            });
         });
         describe('.migrationNeeded()', () => {
             it('return true if schema-version is 0', async () => {

test/unit/rx-database.test.ts

@@ -411,6 +411,37 @@ describe('rx-database.test.ts', () => {
 
                 await db2.close();
             });
+            it('should be able to add the collection with an adjusted schema after the first creation failed', async () => {
+                const db = await createRxDatabase({
+                    name: randomToken(10),
+                    storage: config.storage.getStorage()
+                });
+                await AsyncTestUtil.assertThrows(
+                    () => db.addCollections({
+                        items: {
+                            schema: schemas.human,
+                            statics: {
+                                insert: () => 'foobar'
+                            }
+                        }
+                    }),
+                    'RxError',
+                    'COL17'
+                );
+
+                const collections = await db.addCollections({
+                    items: {
+                        schema: { ...schemas.human, description: 'changed' },
+                        statics: {
+                            foobar: () => 'foobar'
+                        }
+                    }
+                });
+                await collections.items.insert(schemaObjects.humanData());
+                const docs = await collections.items.find().exec();
+                assert.strictEqual(docs.length, 1);
+                await db.remove();
+            });
         });
         describe('negative', () => {
             it('broken schema (nostringIndex)', async () => {