From 5563417a711719cb6e765aff9141b451c079b3a7 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 Jun 2025 18:42:45 -0400 Subject: [PATCH 1/5] Treat "Clear Site Data" event the same way as clearIndexedDbPersistence() from another tab. --- .../firestore/src/core/firestore_client.ts | 35 ++++++++-- packages/firestore/src/local/persistence.ts | 65 ++++++++++++++++++- packages/firestore/src/local/simple_db.ts | 58 +++++++++++------ 3 files changed, 133 insertions(+), 25 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index d04f37a3d7b..137cd83be41 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -231,22 +231,47 @@ export async function setOfflineComponentProvider( } }); - offlineComponentProvider.persistence.setDatabaseDeletedListener(() => { - logWarn('Terminating Firestore due to IndexedDb database deletion'); + offlineComponentProvider.persistence.setDatabaseDeletedListener(event => { + let error: FirestoreError | null; + + if (event.type === 'ClearSiteDataDatabaseDeletedEvent') { + const message = + `Terminating Firestore in response to "${event.type}" event ` + + `to prevent potential IndexedDB database corruption. ` + + `This situation could be caused by clicking the ` + + `"Clear Site Data" button in a web browser. ` + + `Try reloading the web page to re-initialize the ` + + `IndexedDB database.`; + // Throw FirestoreError rather than just Error so that the error will + // be treated as "non-retryable". + error = new FirestoreError('failed-precondition', message); + logWarn(message, event.data); + } else { + error = null; + logWarn( + `Terminating Firestore in response to "${event.type}" event`, + event.data + ); + } + client .terminate() .then(() => { logDebug( - 'Terminating Firestore due to IndexedDb database deletion ' + + `Terminating Firestore in response to "${event.type}" event ` + 'completed successfully' ); }) .catch(error => { logWarn( - 'Terminating Firestore due to IndexedDb database deletion failed', - error + `Terminating Firestore in response to "${event.type}" event failed:`, + error ); }); + + if (error) { + throw error; + } }); client._offlineComponents = offlineComponentProvider; diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts index 113efe7b7d3..fd0c72822e3 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -98,7 +98,70 @@ export interface ReferenceDelegate { ): PersistencePromise; } -export type DatabaseDeletedListener = () => void; +/** + * A {@link DatabaseDeletedListener} event indicating that the IndexedDB + * database received a "versionchange" event with a null value for "newVersion". + * This event indicates that another tab in multi-tab IndexedDB persistence mode + * has called `clearIndexedDbPersistence()` and requires this tab to close its + * IndexedDB connection in order to allow the "clear" operation to proceed. + */ +export class VersionChangeDatabaseDeletedEvent { + /** A type discriminator. */ + readonly type = 'VersionChangeDatabaseDeletedEvent' as const; + + constructor( + readonly data: { + /** + * The value of the "newVersion" property of the "versionchange" event + * that triggered this event. Its value is _always_ `null`, but is kept here + * for posterity. + */ + eventNewVersion: null; + } + ) {} +} + +/** + * A {@link DatabaseDeletedListener} event indicating that the "Clear Site Data" + * button in a web browser was (likely) clicked, deleting the IndexedDB + * database. + */ +export class ClearSiteDataDatabaseDeletedEvent { + /** A type discriminator. */ + readonly type = 'ClearSiteDataDatabaseDeletedEvent' as const; + + constructor( + readonly data: { + /** The IndexedDB version that was last reported by the database. */ + lastClosedVersion: number; + /** + * The value of the "oldVersion" property of the "onupgradeneeded" + * IndexedDB event that triggered this event. + */ + eventOldVersion: number; + /** + * The value of the "newVersion" property of the "onupgradeneeded" + * IndexedDB event that triggered this event. + */ + eventNewVersion: number | null; + /** + * The value of the "version" property of the "IDBDatabase" object. + */ + dbVersion: number; + } + ) {} +} + +/** + * The type of the "event" parameter of {@link DatabaseDeletedListener}. + */ +export type DatabaseDeletedListenerEvent = + | VersionChangeDatabaseDeletedEvent + | ClearSiteDataDatabaseDeletedEvent; + +export type DatabaseDeletedListener = ( + event: DatabaseDeletedListenerEvent +) => void; /** * Persistence is the lowest-level shared interface to persistent storage in diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 1e315c5dae6..8980f9bf091 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -19,10 +19,14 @@ import { getGlobal, getUA, isIndexedDBAvailable } from '@firebase/util'; import { debugAssert } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; -import { logDebug, logError, logWarn } from '../util/log'; +import { logDebug, logError } from '../util/log'; import { Deferred } from '../util/promise'; -import { DatabaseDeletedListener } from './persistence'; +import { + ClearSiteDataDatabaseDeletedEvent, + DatabaseDeletedListener, + VersionChangeDatabaseDeletedEvent +} from './persistence'; import { PersistencePromise } from './persistence_promise'; // References to `indexedDB` are guarded by SimpleDb.isAvailable() and getGlobal() @@ -299,8 +303,31 @@ export class SimpleDb { // https://developer.mozilla.org/en-US/docs/Web/API/IDBVersionChangeRequest/setVersion const request = indexedDB.open(this.name, this.version); + // Store information about "Clear Site Data" being detected in the + // "onupgradeneeded" event and check it in the "onsuccess" event + // rather than throwing directly from the "onupgradeneeded" event + // since throwing directly from the listener results in a generic + // exception that cannot be distinguished from other errors. + const clearSiteDataEvent = { + event: null as ClearSiteDataDatabaseDeletedEvent | null + }; + request.onsuccess = (event: Event) => { const db = (event.target as IDBOpenDBRequest).result; + + if (clearSiteDataEvent.event) { + try { + this.databaseDeletedListener?.(clearSiteDataEvent.event); + } catch (e) { + try { + db.close(); + } finally { + reject(e); + } + return; + } + } + resolve(db); }; @@ -353,19 +380,12 @@ export class SimpleDb { this.lastClosedDbVersion !== null && this.lastClosedDbVersion !== event.oldVersion ) { - // This thrown error will get passed to the `onerror` callback - // registered above, and will then be propagated correctly. - throw new Error( - `refusing to open IndexedDB database due to potential ` + - `corruption of the IndexedDB database data; this corruption ` + - `could be caused by clicking the "clear site data" button in ` + - `a web browser; try reloading the web page to re-initialize ` + - `the IndexedDB database: ` + - `lastClosedDbVersion=${this.lastClosedDbVersion}, ` + - `event.oldVersion=${event.oldVersion}, ` + - `event.newVersion=${event.newVersion}, ` + - `db.version=${db.version}` - ); + clearSiteDataEvent.event = new ClearSiteDataDatabaseDeletedEvent({ + lastClosedVersion: this.lastClosedDbVersion, + eventOldVersion: event.oldVersion, + eventNewVersion: event.newVersion, + dbVersion: db.version + }); } this.schemaConverter .createOrUpgrade( @@ -399,11 +419,11 @@ export class SimpleDb { // Notify the listener if another tab attempted to delete the IndexedDb // database, such as by calling clearIndexedDbPersistence(). if (event.newVersion === null) { - logWarn( - `Received "versionchange" event with newVersion===null; ` + - 'notifying the registered DatabaseDeletedListener, if any' + this.databaseDeletedListener?.( + new VersionChangeDatabaseDeletedEvent({ + eventNewVersion: event.newVersion + }) ); - this.databaseDeletedListener?.(); } }, { passive: true } From cd84a44495a09b05cd2c6f1137875b311fc130b7 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 Jun 2025 19:03:24 -0400 Subject: [PATCH 2/5] yarn changeset --- .changeset/hot-birds-report.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/hot-birds-report.md diff --git a/.changeset/hot-birds-report.md b/.changeset/hot-birds-report.md new file mode 100644 index 00000000000..7dcc7092d4d --- /dev/null +++ b/.changeset/hot-birds-report.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': patch +--- + +Terminate Firestore more gracefully when "Clear Site Data" button is pressed in a web browser From 9482f208a477df8fe262f1aa4c67f6edaf85eabb Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 Jun 2025 19:08:06 -0400 Subject: [PATCH 3/5] yarn format --- packages/firestore/src/core/firestore_client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 137cd83be41..b162a271ed1 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -265,7 +265,7 @@ export async function setOfflineComponentProvider( .catch(error => { logWarn( `Terminating Firestore in response to "${event.type}" event failed:`, - error + error ); }); From 37803ddd6c893553ff875be0ef5f19d0881745f6 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 Jun 2025 10:27:25 -0400 Subject: [PATCH 4/5] code cleanup --- .../firestore/src/core/firestore_client.ts | 19 +++---- packages/firestore/src/local/persistence.ts | 4 +- packages/firestore/src/local/simple_db.ts | 51 ++++++++++--------- 3 files changed, 39 insertions(+), 35 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index b162a271ed1..3b331dd788f 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -235,17 +235,18 @@ export async function setOfflineComponentProvider( let error: FirestoreError | null; if (event.type === 'ClearSiteDataDatabaseDeletedEvent') { - const message = - `Terminating Firestore in response to "${event.type}" event ` + - `to prevent potential IndexedDB database corruption. ` + - `This situation could be caused by clicking the ` + - `"Clear Site Data" button in a web browser. ` + - `Try reloading the web page to re-initialize the ` + - `IndexedDB database.`; // Throw FirestoreError rather than just Error so that the error will // be treated as "non-retryable". - error = new FirestoreError('failed-precondition', message); - logWarn(message, event.data); + error = new FirestoreError( + 'failed-precondition', + `Terminating Firestore in response to "${event.type}" event ` + + `to prevent potential IndexedDB database corruption. ` + + `This situation could be caused by clicking the ` + + `"Clear Site Data" button in a web browser. ` + + `Try reloading the web page to re-initialize the ` + + `IndexedDB database.` + ); + logWarn(error.message, event.data); } else { error = null; logWarn( diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts index fd0c72822e3..03bc6f5ae2a 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -113,8 +113,8 @@ export class VersionChangeDatabaseDeletedEvent { readonly data: { /** * The value of the "newVersion" property of the "versionchange" event - * that triggered this event. Its value is _always_ `null`, but is kept here - * for posterity. + * that triggered this event. Its value is _always_ `null`, but is kept + * here for posterity. */ eventNewVersion: null; } diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 8980f9bf091..6f2e4add7bd 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -304,31 +304,32 @@ export class SimpleDb { const request = indexedDB.open(this.name, this.version); // Store information about "Clear Site Data" being detected in the - // "onupgradeneeded" event and check it in the "onsuccess" event - // rather than throwing directly from the "onupgradeneeded" event - // since throwing directly from the listener results in a generic - // exception that cannot be distinguished from other errors. - const clearSiteDataEvent = { - event: null as ClearSiteDataDatabaseDeletedEvent | null - }; + // "onupgradeneeded" event listener and handle it in the "onsuccess" + // event listener, as opposed to throwing directly from the + // "onupgradeneeded" event listener. Do this because throwing from the + // "onupgradeneeded" event listener results in a generic error being + // reported to the "onerror" event listener that cannot be distinguished + // from other errors. + const clearSiteDataEvent: ClearSiteDataDatabaseDeletedEvent[] = []; request.onsuccess = (event: Event) => { - const db = (event.target as IDBOpenDBRequest).result; - - if (clearSiteDataEvent.event) { + let error: unknown; + if (clearSiteDataEvent[0]) { try { - this.databaseDeletedListener?.(clearSiteDataEvent.event); + this.databaseDeletedListener?.(clearSiteDataEvent[0]); } catch (e) { - try { - db.close(); - } finally { - reject(e); - } - return; + error = e; } } - resolve(db); + const db = (event.target as IDBOpenDBRequest).result; + + if (error) { + reject(error); + db.close(); + } else { + resolve(db); + } }; request.onblocked = () => { @@ -380,12 +381,14 @@ export class SimpleDb { this.lastClosedDbVersion !== null && this.lastClosedDbVersion !== event.oldVersion ) { - clearSiteDataEvent.event = new ClearSiteDataDatabaseDeletedEvent({ - lastClosedVersion: this.lastClosedDbVersion, - eventOldVersion: event.oldVersion, - eventNewVersion: event.newVersion, - dbVersion: db.version - }); + clearSiteDataEvent.push( + new ClearSiteDataDatabaseDeletedEvent({ + lastClosedVersion: this.lastClosedDbVersion, + eventOldVersion: event.oldVersion, + eventNewVersion: event.newVersion, + dbVersion: db.version + }) + ); } this.schemaConverter .createOrUpgrade( From 51a279d358d6c0b01725bf2de2b2a506327a5947 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 Jun 2025 10:42:20 -0400 Subject: [PATCH 5/5] more cleanup, and add eventId to each event --- packages/firestore/src/core/firestore_client.ts | 9 +++++---- packages/firestore/src/local/persistence.ts | 4 ++++ packages/firestore/src/local/simple_db.ts | 3 +++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 3b331dd788f..c18aa27f28b 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -232,7 +232,7 @@ export async function setOfflineComponentProvider( }); offlineComponentProvider.persistence.setDatabaseDeletedListener(event => { - let error: FirestoreError | null; + let error: FirestoreError | undefined; if (event.type === 'ClearSiteDataDatabaseDeletedEvent') { // Throw FirestoreError rather than just Error so that the error will @@ -248,7 +248,6 @@ export async function setOfflineComponentProvider( ); logWarn(error.message, event.data); } else { - error = null; logWarn( `Terminating Firestore in response to "${event.type}" event`, event.data @@ -260,13 +259,15 @@ export async function setOfflineComponentProvider( .then(() => { logDebug( `Terminating Firestore in response to "${event.type}" event ` + - 'completed successfully' + 'completed successfully', + event.data ); }) .catch(error => { logWarn( `Terminating Firestore in response to "${event.type}" event failed:`, - error + error, + event.data ); }); diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts index 03bc6f5ae2a..5cf580a4cd0 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -111,6 +111,8 @@ export class VersionChangeDatabaseDeletedEvent { constructor( readonly data: { + /** A unique ID for this event. */ + eventId: string; /** * The value of the "newVersion" property of the "versionchange" event * that triggered this event. Its value is _always_ `null`, but is kept @@ -132,6 +134,8 @@ export class ClearSiteDataDatabaseDeletedEvent { constructor( readonly data: { + /** A unique ID for this event. */ + eventId: string; /** The IndexedDB version that was last reported by the database. */ lastClosedVersion: number; /** diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 6f2e4add7bd..7cedad217fd 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -18,6 +18,7 @@ import { getGlobal, getUA, isIndexedDBAvailable } from '@firebase/util'; import { debugAssert } from '../util/assert'; +import { generateUniqueDebugId } from '../util/debug_uid'; import { Code, FirestoreError } from '../util/error'; import { logDebug, logError } from '../util/log'; import { Deferred } from '../util/promise'; @@ -383,6 +384,7 @@ export class SimpleDb { ) { clearSiteDataEvent.push( new ClearSiteDataDatabaseDeletedEvent({ + eventId: generateUniqueDebugId(), lastClosedVersion: this.lastClosedDbVersion, eventOldVersion: event.oldVersion, eventNewVersion: event.newVersion, @@ -424,6 +426,7 @@ export class SimpleDb { if (event.newVersion === null) { this.databaseDeletedListener?.( new VersionChangeDatabaseDeletedEvent({ + eventId: generateUniqueDebugId(), eventNewVersion: event.newVersion }) );