Skip to content

Commit 6673a92

Browse files
author
Michael Lehenbauer
authored
Provide warning and potential workaround for iOS Safari bug described by #1670. (#1854)
* On iOS 12.2 we proactively warn about the "internal error was encountered in Indexed Database Server" Bug. * When the bug is actually encountered on iOS >= 12.2 an < 13 we log a custom error message. Both log message and error link to https://stackoverflow.com/q/56496296/110915 which provides potential error handling / recovery example.
1 parent d164cac commit 6673a92

File tree

3 files changed

+66
-12
lines changed

3 files changed

+66
-12
lines changed

packages/firestore/CHANGELOG.md

+5-2
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,17 @@
22
# Unreleased
33
- [feature] Added `clearPersistence()`, which clears the persistent storage
44
including pending writes and cached documents. This is intended to help
5-
write reliable tests (#449).
5+
write reliable tests (#449).
6+
- [changed] Added logging and a custom error message to help users hitting
7+
https://bugs.webkit.org/show_bug.cgi?id=197050 (a bug in iOS 12.2 causing
8+
the SDK to potentially crash when persistence is enabled).
69

710
# 1.3.3
811
- [changed] Firestore now recovers more quickly after network connectivity
912
changes (airplane mode, Wi-Fi availability, etc.).
1013

1114
# 1.3.0
12-
- [changed] Deprecated the `experimentalTabSynchronization` setting in favor of
15+
- [changed] Deprecated the `experimentalTabSynchronization` setting in favor of
1316
`synchronizeTabs`. If you use multi-tab synchronization, it is recommended
1417
that you update your call to `enablePersistence()`. Firestore logs an error
1518
if you continue to use `experimentalTabSynchronization`.

packages/firestore/src/local/simple_db.ts

+59-8
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { getUA } from '@firebase/util';
1819
import { assert } from '../util/assert';
1920
import { Code, FirestoreError } from '../util/error';
20-
import { debug } from '../util/log';
21+
import { debug, error } from '../util/log';
2122
import { Deferred } from '../util/promise';
2223
import { SCHEMA_VERSION } from './indexeddb_schema';
2324
import { PersistencePromise } from './persistence_promise';
@@ -149,8 +150,7 @@ export class SimpleDb {
149150
}
150151

151152
// Check the UA string to find out the browser.
152-
// TODO(mikelehen): Move this logic into packages/util/environment.ts
153-
const ua = window.navigator.userAgent;
153+
const ua = getUA();
154154

155155
// IE 10
156156
// ua = 'Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.2; Trident/6.0)';
@@ -195,7 +195,12 @@ export class SimpleDb {
195195
/** Parse User Agent to determine iOS version. Returns -1 if not found. */
196196
static getIOSVersion(ua: string): number {
197197
const iOSVersionRegex = ua.match(/i(?:phone|pad|pod) os ([\d_]+)/i);
198-
const version = iOSVersionRegex ? iOSVersionRegex[1].split('_')[0] : '-1';
198+
const version = iOSVersionRegex
199+
? iOSVersionRegex[1]
200+
.split('_')
201+
.slice(0, 2)
202+
.join('.')
203+
: '-1';
199204
return Number(version);
200205
}
201206

@@ -212,7 +217,21 @@ export class SimpleDb {
212217
return Number(version);
213218
}
214219

215-
constructor(private db: IDBDatabase) {}
220+
constructor(private db: IDBDatabase) {
221+
const iOSVersion = SimpleDb.getIOSVersion(getUA());
222+
// NOTE: According to https://bugs.webkit.org/show_bug.cgi?id=197050, the
223+
// bug we're checking for should exist in iOS >= 12.2 and < 13, but for
224+
// whatever reason it's much harder to hit after 12.2 so we only proactively
225+
// log on 12.2.
226+
if (iOSVersion === 12.2) {
227+
error(
228+
'Firestore persistence suffers from a bug in iOS 12.2 ' +
229+
'Safari that may cause your app to stop working. See ' +
230+
'https://stackoverflow.com/q/56496296/110915 for details ' +
231+
'and a potential workaround.'
232+
);
233+
}
234+
}
216235

217236
setVersionChangeListener(
218237
versionChangeListener: (event: IDBVersionChangeEvent) => void
@@ -359,7 +378,9 @@ export class SimpleDbTransaction {
359378
}
360379
};
361380
this.transaction.onerror = (event: Event) => {
362-
this.completionDeferred.reject((event.target as IDBRequest).error!);
381+
const error = (event.target as IDBRequest).error!;
382+
checkForAndReportiOSError(error);
383+
this.completionDeferred.reject(error);
363384
};
364385
}
365386

@@ -578,7 +599,9 @@ export class SimpleDbStore<
578599
const cursorRequest = this.cursor({});
579600
return new PersistencePromise((resolve, reject) => {
580601
cursorRequest.onerror = (event: Event) => {
581-
reject((event.target as IDBRequest).error!);
602+
const error = (event.target as IDBRequest).error!;
603+
checkForAndReportiOSError(error);
604+
reject(error);
582605
};
583606
cursorRequest.onsuccess = (event: Event) => {
584607
const cursor: IDBCursorWithValue = (event.target as IDBRequest).result;
@@ -692,7 +715,35 @@ function wrapRequest<R>(request: IDBRequest): PersistencePromise<R> {
692715
};
693716

694717
request.onerror = (event: Event) => {
695-
reject((event.target as IDBRequest).error!);
718+
const error = (event.target as IDBRequest).error!;
719+
checkForAndReportiOSError(error);
720+
reject(error);
696721
};
697722
});
698723
}
724+
725+
// Guard so we only report the error once.
726+
let reportedIOSError = false;
727+
function checkForAndReportiOSError(error: DOMException): void {
728+
if (reportedIOSError) {
729+
return;
730+
}
731+
const iOSVersion = SimpleDb.getIOSVersion(getUA());
732+
if (iOSVersion >= 12.2 && iOSVersion < 13) {
733+
const IOS_ERROR =
734+
'An internal error was encountered in the Indexed Database server';
735+
if (error.message.indexOf(IOS_ERROR) >= 0) {
736+
reportedIOSError = true;
737+
// Throw a global exception outside of this promise chain, for the user to
738+
// potentially catch.
739+
setTimeout(() => {
740+
throw new FirestoreError(
741+
'internal',
742+
`IOS_INDEXEDDB_BUG1: IndexedDb has thrown '${IOS_ERROR}'. This is likely ` +
743+
`due to an unavoidable bug in iOS. See https://stackoverflow.com/q/56496296/110915 ` +
744+
`for details and a potential workaround.`
745+
);
746+
}, 0);
747+
}
748+
}
749+
}

packages/firestore/test/unit/local/simple_db.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ describe('SimpleDb', () => {
132132
const androidAgent =
133133
'Mozilla/5.0 (Linux; U; Android 2.2.1; fr-fr; Desire HD Build/FRG83D)' +
134134
' AppleWebKit/533.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1';
135-
expect(SimpleDb.getIOSVersion(iPhoneSafariAgent)).to.equal(10);
136-
expect(SimpleDb.getIOSVersion(iPadSafariAgent)).to.equal(9);
135+
expect(SimpleDb.getIOSVersion(iPhoneSafariAgent)).to.equal(10.14);
136+
expect(SimpleDb.getIOSVersion(iPadSafariAgent)).to.equal(9.0);
137137
expect(SimpleDb.getAndroidVersion(androidAgent)).to.equal(2.2);
138138
});
139139

0 commit comments

Comments
 (0)