From 5563417a711719cb6e765aff9141b451c079b3a7 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 Jun 2025 18:42:45 -0400 Subject: [PATCH 01/13] 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 02/13] 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 03/13] 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 04/13] 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 05/13] 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 }) ); From a40528cf3eb56b3d7de8ef3eedb3968274edc511 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 10 Jul 2025 11:20:04 -0400 Subject: [PATCH 06/13] touch firestore/src/api/database.ts --- packages/firestore/src/api/database.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index a2feb19507f..6ff19f7a87c 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -269,6 +269,7 @@ export function getFirestore( connectFirestoreEmulator(db, ...emulator); } } + return db; } From 61178d5e2760764aa89bfde260c930859c324ec6 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Fri, 9 May 2025 10:13:11 -0600 Subject: [PATCH 07/13] Enable contextual debug logging for integration tests --- packages/firestore/src/util/log.ts | 191 +++++++++++++++++- .../test/integration/util/firebase_export.ts | 5 +- 2 files changed, 190 insertions(+), 6 deletions(-) diff --git a/packages/firestore/src/util/log.ts b/packages/firestore/src/util/log.ts index cbdbad38ecf..70f36ed2558 100644 --- a/packages/firestore/src/util/log.ts +++ b/packages/firestore/src/util/log.ts @@ -15,14 +15,18 @@ * limitations under the License. */ -import { Logger, LogLevel, LogLevelString } from '@firebase/logger'; +import { Logger, LogHandler, LogLevel, LogLevelString } from '@firebase/logger'; import { SDK_VERSION } from '../core/version'; import { formatJSON } from '../platform/format_json'; +import { generateUniqueDebugId } from './debug_uid'; + export { LogLevel, LogLevelString }; const logClient = new Logger('@firebase/firestore'); +const defaultLogHandler = logClient.logHandler; +let logBuffer: LogBuffer | undefined; // Helper methods are needed because variables can't be exported as read/write export function getLogLevel(): LogLevel { @@ -41,20 +45,37 @@ export function getLogLevel(): LogLevel { *
  • `error` to log errors only.
  • *
  • `silent` to turn off logging.
  • * + * @param includeContext - If set to a positive value, the logger will buffer + * all log messages (of all log levels) and log the most recent messages + * when a message of `logLevel` is seen. This is useful if you want to get + * debug logging from the SDK leading up to a warning or error, but do not + * always want debug log verbosity. This param specifies how many messages + * to buffer. */ -export function setLogLevel(logLevel: LogLevelString): void { +export function setLogLevel( + logLevel: LogLevelString, + includeContext: number = 0 +): void { logClient.setLogLevel(logLevel); + + if (includeContext > 0) { + logBuffer = new LogBuffer(includeContext); + logClient.logHandler = bufferingLogHandler; + } else { + logBuffer = undefined; + logClient.logHandler = defaultLogHandler; + } } export function logDebug(msg: string, ...obj: unknown[]): void { - if (logClient.logLevel <= LogLevel.DEBUG) { + if (logBuffer || logClient.logLevel <= LogLevel.DEBUG) { const args = obj.map(argToString); logClient.debug(`Firestore (${SDK_VERSION}): ${msg}`, ...args); } } export function logError(msg: string, ...obj: unknown[]): void { - if (logClient.logLevel <= LogLevel.ERROR) { + if (logBuffer || logClient.logLevel <= LogLevel.ERROR) { const args = obj.map(argToString); logClient.error(`Firestore (${SDK_VERSION}): ${msg}`, ...args); } @@ -64,7 +85,7 @@ export function logError(msg: string, ...obj: unknown[]): void { * @internal */ export function logWarn(msg: string, ...obj: unknown[]): void { - if (logClient.logLevel <= LogLevel.WARN) { + if (logBuffer || logClient.logLevel <= LogLevel.WARN) { const args = obj.map(argToString); logClient.warn(`Firestore (${SDK_VERSION}): ${msg}`, ...args); } @@ -85,3 +106,163 @@ function argToString(obj: unknown): string | unknown { } } } + +class LogBuffer { + private _buffer: Array<{ level: LogLevel; now: string; args: unknown[] }>; + private _numTruncated: number = 0; + + constructor(readonly bufferSize: number) { + this._buffer = []; + this._numTruncated = 0; + } + + /** + * Clear the log buffer + */ + clear(): void { + this._buffer = []; + this._numTruncated = 0; + } + + /** + * Add a new log message to the buffer. If the buffer will exceed + * the allocated buffer size, then remove the oldest message from + * the buffer. + * @param level + * @param now + * @param args + */ + add(level: LogLevel, now: string, args: unknown[]): void { + this._buffer.push({ + level, + now, + args + }); + + if (this._buffer.length > this.bufferSize) { + // remove the first (oldest) element + this._buffer.shift(); + this._numTruncated++; + } + } + + /** + * Returns the number of old log messages that have been + * truncated from the log to maintain buffer size. + */ + get numTruncated(): number { + return this._numTruncated; + } + + get first(): { level: LogLevel; now: string; args: unknown[] } | undefined { + return this._buffer[0]; + } + + /** + * Iterate from oldest to newest. + */ + [Symbol.iterator](): Iterator<{ + level: LogLevel; + now: string; + args: unknown[]; + }> { + let currentIndex = 0; + // Create a snapshot of the buffer for iteration. + // This ensures that if the buffer is modified while iterating (e.g., by adding new logs), + // the iterator will continue to iterate over the state of the buffer as it was when iteration began. + // It also means you iterate from the oldest to the newest log. + const bufferSnapshot = [...this._buffer]; + + return { + next: (): IteratorResult<{ + level: LogLevel; + now: string; + args: unknown[]; + }> => { + if (currentIndex < bufferSnapshot.length) { + return { value: bufferSnapshot[currentIndex++], done: false }; + } else { + return { value: undefined, done: true }; + } + } + }; + } +} + +/** + * By default, `console.debug` is not displayed in the developer console (in + * chrome). To avoid forcing users to have to opt-in to these logs twice + * (i.e. once for firebase, and once in the console), we are sending `DEBUG` + * logs to the `console.log` function. + */ +const ConsoleMethod = { + [LogLevel.DEBUG]: 'log', + [LogLevel.VERBOSE]: 'log', + [LogLevel.INFO]: 'info', + [LogLevel.WARN]: 'warn', + [LogLevel.ERROR]: 'error' +}; + +/** + * The default log handler will forward DEBUG, VERBOSE, INFO, WARN, and ERROR + * messages on to their corresponding console counterparts (if the log method + * is supported by the current log level) + */ +const bufferingLogHandler: LogHandler = (instance, logType, ...args): void => { + const now = new Date().toISOString(); + + // Fail-safe. This is never expected to be true, but if it is, + // it's not important enough to throw. + if (!logBuffer) { + defaultLogHandler(instance, logType, args); + return; + } + + // Buffer any messages less than the current logLevel + if (logType < instance.logLevel) { + logBuffer!.add(logType, now, args); + return; + } + + // create identifier that associates all of the associated + // context messages with the log message that caused the + // flush of the logBuffer + const id = generateUniqueDebugId(); + + // Optionally write a log message stating if any log messages + // were skipped. + if (logBuffer.first) { + writeLog(instance, id, LogLevel.INFO, logBuffer.first.now, [ + `... ${logBuffer.numTruncated} log messages skipped ...` + ]); + } + + // If here, write the log buffer contents as context + for (const logInfo of logBuffer) { + writeLog(instance, id, logInfo.level, logInfo.now, logInfo.args); + } + logBuffer.clear(); + + // Now write the target log message. + writeLog(instance, id, logType, now, args); +}; + +function writeLog( + instance: Logger, + id: string, + logType: LogLevel, + now: string, + args: unknown[] +): void { + const method = ConsoleMethod[logType as keyof typeof ConsoleMethod]; + if (method) { + console[method as 'log' | 'info' | 'warn' | 'error']( + `[${now}] (context: ${id}) ${instance.name}:`, + ...args + ); + } else { + throw new Error( + `Attempted to log a message with an invalid logType (value: ${logType})` + ); + } +} diff --git a/packages/firestore/test/integration/util/firebase_export.ts b/packages/firestore/test/integration/util/firebase_export.ts index f58b3ce045b..0f601bd7b26 100644 --- a/packages/firestore/test/integration/util/firebase_export.ts +++ b/packages/firestore/test/integration/util/firebase_export.ts @@ -22,13 +22,16 @@ import { FirebaseApp, initializeApp } from '@firebase/app'; -import { Firestore, initializeFirestore } from '../../../src'; +import { Firestore, initializeFirestore, setLogLevel } from '../../../src'; import { PrivateSettings } from '../../../src/lite-api/settings'; // TODO(dimond): Right now we create a new app and Firestore instance for // every test and never clean them up. We may need to revisit. let appCount = 0; +// enable contextual debug logging +setLogLevel('error', 100); + export function newTestApp(projectId: string, appName?: string): FirebaseApp { if (appName === undefined) { appName = 'test-app-' + appCount++; From d039e436b4b037d63a5375d22eb40a3932606188 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Thu, 10 Jul 2025 15:14:14 -0600 Subject: [PATCH 08/13] Enable contextual debug logging to help diagnose CI failures --- packages/firestore/src/api/database.ts | 2 +- packages/firestore/src/local/simple_db.ts | 5 +++-- packages/firestore/src/util/log.ts | 13 +++++++++++++ .../test/integration/util/firebase_export.ts | 2 +- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 6ff19f7a87c..3225d826126 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -269,7 +269,7 @@ export function getFirestore( connectFirestoreEmulator(db, ...emulator); } } - + return db; } diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 1e315c5dae6..81dea4af619 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -17,7 +17,7 @@ import { getGlobal, getUA, isIndexedDBAvailable } from '@firebase/util'; -import { debugAssert } from '../util/assert'; +import { debugAssert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { logDebug, logError, logWarn } from '../util/log'; import { Deferred } from '../util/promise'; @@ -355,7 +355,8 @@ export class SimpleDb { ) { // This thrown error will get passed to the `onerror` callback // registered above, and will then be propagated correctly. - throw new Error( + fail( + 0x3456, `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 ` + diff --git a/packages/firestore/src/util/log.ts b/packages/firestore/src/util/log.ts index 70f36ed2558..8a778dd0822 100644 --- a/packages/firestore/src/util/log.ts +++ b/packages/firestore/src/util/log.ts @@ -224,6 +224,19 @@ const bufferingLogHandler: LogHandler = (instance, logType, ...args): void => { return; } + let codeFound = false; + args.forEach(v => { + if (typeof v === 'string' && /ID:\s3456/.test(v)) { + codeFound = true; + } + }); + + // Buffer any message that do not match the expected code + if (!codeFound) { + logBuffer!.add(logType, now, args); + return; + } + // create identifier that associates all of the associated // context messages with the log message that caused the // flush of the logBuffer diff --git a/packages/firestore/test/integration/util/firebase_export.ts b/packages/firestore/test/integration/util/firebase_export.ts index 0f601bd7b26..f8882dadd0e 100644 --- a/packages/firestore/test/integration/util/firebase_export.ts +++ b/packages/firestore/test/integration/util/firebase_export.ts @@ -30,7 +30,7 @@ import { PrivateSettings } from '../../../src/lite-api/settings'; let appCount = 0; // enable contextual debug logging -setLogLevel('error', 100); +setLogLevel('error', 1000); export function newTestApp(projectId: string, appName?: string): FirebaseApp { if (appName === undefined) { From 3f24c15678fd78d1f866ae096e532dc8dab8a97e Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Thu, 10 Jul 2025 18:03:32 -0400 Subject: [PATCH 09/13] src/local/simple_db.ts: check `clearSiteDataEvent.length > 0` instead of `clearSiteDataEvent[0]` --- packages/firestore/src/local/simple_db.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 7cedad217fd..1169d2c9b54 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -315,7 +315,7 @@ export class SimpleDb { request.onsuccess = (event: Event) => { let error: unknown; - if (clearSiteDataEvent[0]) { + if (clearSiteDataEvent.length > 0) { try { this.databaseDeletedListener?.(clearSiteDataEvent[0]); } catch (e) { From 509171f4fb1ac33918346ef75c39740b00fdad74 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Thu, 10 Jul 2025 18:37:26 -0400 Subject: [PATCH 10/13] yarn format --- packages/firestore/src/local/simple_db.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index b47756175bc..67073eccaed 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -319,7 +319,10 @@ export class SimpleDb { try { this.databaseDeletedListener?.(clearSiteDataEvent[0]); } catch (e) { - logError(`zzyzx this.databaseDeletedListener?.(clearSiteDataEvent[0]) threw`, e); + logError( + `zzyzx this.databaseDeletedListener?.(clearSiteDataEvent[0]) threw`, + e + ); error = e; } } From 0e357ba9d7b4a18a7e68e4b0d4b155cea80cd339 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Thu, 10 Jul 2025 18:42:29 -0400 Subject: [PATCH 11/13] simple_db: add more debug logging --- packages/firestore/src/local/simple_db.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 67073eccaed..f63c9854418 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -166,6 +166,7 @@ export class SimpleDb { private db?: IDBDatabase; private databaseDeletedListener?: DatabaseDeletedListener; private lastClosedDbVersion: number | null = null; + private readonly logTag = `${LOG_TAG} [${generateUniqueDebugId()}]`; /** Deletes the specified database. */ static delete(name: string): Promise { @@ -270,6 +271,7 @@ export class SimpleDb { private readonly version: number, private readonly schemaConverter: SimpleDbSchemaConverter ) { + logDebug(`${this.logTag} created!`); debugAssert( SimpleDb.isAvailable(), 'IndexedDB not supported in current environment.' @@ -295,7 +297,8 @@ export class SimpleDb { */ async ensureDb(action: string): Promise { if (!this.db) { - logDebug(LOG_TAG, 'Opening database:', this.name); + console.trace("zzyzx SimpleDb.ensureDb() is opening a new database connection"); + logDebug(this.logTag, 'Opening database:', this.name); this.db = await new Promise((resolve, reject) => { // TODO(mikelehen): Investigate browser compatibility. // https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB @@ -377,7 +380,7 @@ export class SimpleDb { request.onupgradeneeded = (event: IDBVersionChangeEvent) => { logDebug( - LOG_TAG, + this.logTag, 'Database "' + this.name + '" requires upgrade from version:', event.oldVersion ); @@ -405,7 +408,7 @@ export class SimpleDb { ) .next(() => { logDebug( - LOG_TAG, + this.logTag, 'Database upgrade to version ' + this.version + ' complete' ); }); @@ -415,6 +418,7 @@ export class SimpleDb { this.db.addEventListener( 'close', event => { + logDebug(`${this.logTag} close callback`); const db = event.target as IDBDatabase; this.lastClosedDbVersion = db.version; }, @@ -460,6 +464,7 @@ export class SimpleDb { objectStores: string[], transactionFn: (transaction: SimpleDbTransaction) => PersistencePromise ): Promise { + logDebug(`${this.logTag} runTransaction action=${action}`); const readonly = mode === 'readonly'; let attemptNumber = 0; @@ -512,7 +517,7 @@ export class SimpleDb { error.name !== 'FirebaseError' && attemptNumber < TRANSACTION_RETRY_COUNT; logDebug( - LOG_TAG, + this.logTag, 'Transaction failed with error:', error.message, 'Retrying:', @@ -529,6 +534,7 @@ export class SimpleDb { } close(): void { + logDebug(`${this.logTag} close() called!`); if (this.db) { this.db.close(); } From be1fcd52e4a0c2ff5b3e6fb650384437d9fc727e Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Thu, 10 Jul 2025 18:44:29 -0400 Subject: [PATCH 12/13] simple_db.ts: include the event in the log message --- packages/firestore/src/local/simple_db.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index f63c9854418..93c4a65b0ac 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -418,7 +418,7 @@ export class SimpleDb { this.db.addEventListener( 'close', event => { - logDebug(`${this.logTag} close callback`); + logDebug(`${this.logTag} close callback`, event); const db = event.target as IDBDatabase; this.lastClosedDbVersion = db.version; }, From 390a472eec74ff50272ffa10fed08802ae66f353 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Fri, 11 Jul 2025 13:05:35 -0400 Subject: [PATCH 13/13] revert changes by dconeybe on this branch --- .changeset/hot-birds-report.md | 5 -- .../firestore/src/core/firestore_client.ts | 39 ++------- packages/firestore/src/local/persistence.ts | 69 +-------------- packages/firestore/src/local/simple_db.ts | 85 ++++++------------- 4 files changed, 32 insertions(+), 166 deletions(-) delete mode 100644 .changeset/hot-birds-report.md diff --git a/.changeset/hot-birds-report.md b/.changeset/hot-birds-report.md deleted file mode 100644 index 7dcc7092d4d..00000000000 --- a/.changeset/hot-birds-report.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@firebase/firestore': patch ---- - -Terminate Firestore more gracefully when "Clear Site Data" button is pressed in a web browser diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index c18aa27f28b..d04f37a3d7b 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -231,49 +231,22 @@ export async function setOfflineComponentProvider( } }); - offlineComponentProvider.persistence.setDatabaseDeletedListener(event => { - let error: FirestoreError | undefined; - - if (event.type === 'ClearSiteDataDatabaseDeletedEvent') { - // Throw FirestoreError rather than just Error so that the error will - // be treated as "non-retryable". - 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 { - logWarn( - `Terminating Firestore in response to "${event.type}" event`, - event.data - ); - } - + offlineComponentProvider.persistence.setDatabaseDeletedListener(() => { + logWarn('Terminating Firestore due to IndexedDb database deletion'); client .terminate() .then(() => { logDebug( - `Terminating Firestore in response to "${event.type}" event ` + - 'completed successfully', - event.data + 'Terminating Firestore due to IndexedDb database deletion ' + + 'completed successfully' ); }) .catch(error => { logWarn( - `Terminating Firestore in response to "${event.type}" event failed:`, - error, - event.data + 'Terminating Firestore due to IndexedDb database deletion 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 5cf580a4cd0..113efe7b7d3 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -98,74 +98,7 @@ export interface ReferenceDelegate { ): PersistencePromise; } -/** - * 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: { - /** 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 - * 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: { - /** A unique ID for this event. */ - eventId: string; - /** 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; +export type DatabaseDeletedListener = () => 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 93c4a65b0ac..81dea4af619 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -17,17 +17,12 @@ import { getGlobal, getUA, isIndexedDBAvailable } from '@firebase/util'; -import { debugAssert } from '../util/assert'; -import { generateUniqueDebugId } from '../util/debug_uid'; +import { debugAssert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; -import { logDebug, logError } from '../util/log'; +import { logDebug, logError, logWarn } from '../util/log'; import { Deferred } from '../util/promise'; -import { - ClearSiteDataDatabaseDeletedEvent, - DatabaseDeletedListener, - VersionChangeDatabaseDeletedEvent -} from './persistence'; +import { DatabaseDeletedListener } from './persistence'; import { PersistencePromise } from './persistence_promise'; // References to `indexedDB` are guarded by SimpleDb.isAvailable() and getGlobal() @@ -166,7 +161,6 @@ export class SimpleDb { private db?: IDBDatabase; private databaseDeletedListener?: DatabaseDeletedListener; private lastClosedDbVersion: number | null = null; - private readonly logTag = `${LOG_TAG} [${generateUniqueDebugId()}]`; /** Deletes the specified database. */ static delete(name: string): Promise { @@ -271,7 +265,6 @@ export class SimpleDb { private readonly version: number, private readonly schemaConverter: SimpleDbSchemaConverter ) { - logDebug(`${this.logTag} created!`); debugAssert( SimpleDb.isAvailable(), 'IndexedDB not supported in current environment.' @@ -297,8 +290,7 @@ export class SimpleDb { */ async ensureDb(action: string): Promise { if (!this.db) { - console.trace("zzyzx SimpleDb.ensureDb() is opening a new database connection"); - logDebug(this.logTag, 'Opening database:', this.name); + logDebug(LOG_TAG, 'Opening database:', this.name); this.db = await new Promise((resolve, reject) => { // TODO(mikelehen): Investigate browser compatibility. // https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB @@ -307,37 +299,9 @@ 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 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) => { - let error: unknown; - if (clearSiteDataEvent.length > 0) { - try { - this.databaseDeletedListener?.(clearSiteDataEvent[0]); - } catch (e) { - logError( - `zzyzx this.databaseDeletedListener?.(clearSiteDataEvent[0]) threw`, - e - ); - error = e; - } - } - const db = (event.target as IDBOpenDBRequest).result; - - if (error) { - reject(error); - db.close(); - } else { - resolve(db); - } + resolve(db); }; request.onblocked = () => { @@ -380,7 +344,7 @@ export class SimpleDb { request.onupgradeneeded = (event: IDBVersionChangeEvent) => { logDebug( - this.logTag, + LOG_TAG, 'Database "' + this.name + '" requires upgrade from version:', event.oldVersion ); @@ -389,14 +353,19 @@ export class SimpleDb { this.lastClosedDbVersion !== null && this.lastClosedDbVersion !== event.oldVersion ) { - clearSiteDataEvent.push( - new ClearSiteDataDatabaseDeletedEvent({ - eventId: generateUniqueDebugId(), - lastClosedVersion: this.lastClosedDbVersion, - eventOldVersion: event.oldVersion, - eventNewVersion: event.newVersion, - dbVersion: db.version - }) + // This thrown error will get passed to the `onerror` callback + // registered above, and will then be propagated correctly. + fail( + 0x3456, + `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}` ); } this.schemaConverter @@ -408,7 +377,7 @@ export class SimpleDb { ) .next(() => { logDebug( - this.logTag, + LOG_TAG, 'Database upgrade to version ' + this.version + ' complete' ); }); @@ -418,7 +387,6 @@ export class SimpleDb { this.db.addEventListener( 'close', event => { - logDebug(`${this.logTag} close callback`, event); const db = event.target as IDBDatabase; this.lastClosedDbVersion = db.version; }, @@ -432,12 +400,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) { - this.databaseDeletedListener?.( - new VersionChangeDatabaseDeletedEvent({ - eventId: generateUniqueDebugId(), - eventNewVersion: event.newVersion - }) + logWarn( + `Received "versionchange" event with newVersion===null; ` + + 'notifying the registered DatabaseDeletedListener, if any' ); + this.databaseDeletedListener?.(); } }, { passive: true } @@ -464,7 +431,6 @@ export class SimpleDb { objectStores: string[], transactionFn: (transaction: SimpleDbTransaction) => PersistencePromise ): Promise { - logDebug(`${this.logTag} runTransaction action=${action}`); const readonly = mode === 'readonly'; let attemptNumber = 0; @@ -517,7 +483,7 @@ export class SimpleDb { error.name !== 'FirebaseError' && attemptNumber < TRANSACTION_RETRY_COUNT; logDebug( - this.logTag, + LOG_TAG, 'Transaction failed with error:', error.message, 'Retrying:', @@ -534,7 +500,6 @@ export class SimpleDb { } close(): void { - logDebug(`${this.logTag} close() called!`); if (this.db) { this.db.close(); }