Skip to content

Commit 1119bc3

Browse files
Only retry DOMException/DOMError
1 parent 77cb183 commit 1119bc3

File tree

5 files changed

+52
-11
lines changed

5 files changed

+52
-11
lines changed

packages/firestore/src/util/async_queue.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,10 @@ export class AsyncQueue {
318318
/**
319319
* Enqueue a retryable operation.
320320
*
321-
* A retryable operation is rescheduled with backoff if it fails with any
322-
* exception. All retryable operations are executed in order and only run
323-
* if all prior operations were retried successfully.
321+
* A retryable operation is rescheduled with backoff if it fails with a
322+
* DOMException or a DOMError (the error types used by IndexedDB). All
323+
* retryable operations are executed in order and only run if all prior
324+
* operations were retried successfully.
324325
*/
325326
enqueueRetryable(op: () => Promise<void>): void {
326327
this.verifyNotFailed();
@@ -337,8 +338,19 @@ export class AsyncQueue {
337338
deferred.resolve();
338339
this.backoff.reset();
339340
} catch (e) {
340-
logDebug(LOG_TAG, 'Retryable operation failed: ' + e.message);
341-
this.backoff.backoffAndRun(retryingOp);
341+
// We only retry errors that match the ones thrown by IndexedDB (see
342+
// https://developer.mozilla.org/en-US/docs/Web/API/IDBTransaction/error)
343+
if (
344+
(typeof DOMException !== 'undefined' &&
345+
e instanceof DOMException) ||
346+
(typeof DOMError !== 'undefined' && e instanceof DOMError)
347+
) {
348+
logDebug(LOG_TAG, 'Operation failed with retryable error: ' + e);
349+
this.backoff.backoffAndRun(retryingOp);
350+
} else {
351+
deferred.resolve();
352+
throw e; // Failure will be handled by AsyncQueue
353+
}
342354
}
343355
};
344356
this.enqueueAndForget(retryingOp);

packages/firestore/test/unit/specs/recovery_spec.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describeSpec(
4242
// Client 1 has received the WebStorage notification that the write
4343
// has been acknowledged, but failed to process the change. Hence,
4444
// we did not get a user callback. We schedule the first retry and
45-
// make sure that it also does not get processed until
45+
// make sure that it also does not get processed until
4646
// `recoverDatabase` is called.
4747
.runTimer(TimerId.AsyncQueueRetry)
4848
.recoverDatabase()

packages/firestore/test/unit/specs/spec_test_components.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export class MockPersistence implements Persistence {
113113
) => PersistencePromise<T>
114114
): Promise<T> {
115115
if (this.injectFailures) {
116-
return Promise.reject(new Error('Injected Failure'));
116+
return Promise.reject(new DOMException('Simulated retryable error'));
117117
} else {
118118
return this.delegate.runTransaction(action, mode, transactionOperation);
119119
}

packages/firestore/test/unit/util/async_queue.test.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,16 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { expect } from 'chai';
18+
import * as chaiAsPromised from 'chai-as-promised';
19+
20+
import { expect, use } from 'chai';
1921
import { AsyncQueue, TimerId } from '../../../src/util/async_queue';
2022
import { Code } from '../../../src/util/error';
2123
import { getLogLevel, setLogLevel, LogLevel } from '../../../src/util/log';
2224
import { Deferred, Rejecter, Resolver } from '../../../src/util/promise';
25+
import { fail } from '../../../src/util/assert';
26+
27+
use(chaiAsPromised);
2328

2429
describe('AsyncQueue', () => {
2530
// We reuse these TimerIds for generic testing.
@@ -212,13 +217,29 @@ describe('AsyncQueue', () => {
212217
queue.enqueueRetryable(async () => {
213218
doStep(1);
214219
if (completedSteps.length === 1) {
215-
throw new Error('Simulated retryable error');
220+
throw new DOMException('Simulated retryable error');
216221
}
217222
});
218223
await queue.runDelayedOperationsEarly(TimerId.AsyncQueueRetry);
219224
expect(completedSteps).to.deep.equal([1, 1]);
220225
});
221226

227+
it("Doesn't retry internal exceptions", async () => {
228+
const queue = new AsyncQueue();
229+
// We use a Deferred Promise as retryable operations are scheduled only
230+
// when Promise chains are resolved, which can happen after the
231+
// `queue.enqueue` call below.
232+
const deferred = new Deferred<void>();
233+
queue.enqueueRetryable(async () => {
234+
deferred.resolve();
235+
throw fail('Simulated test failure');
236+
});
237+
await deferred.promise;
238+
await expect(
239+
queue.enqueue(() => Promise.resolve())
240+
).to.eventually.be.rejectedWith('Simulated test failure');
241+
});
242+
222243
it('Schedules first retryable attempt with no delay', async () => {
223244
const queue = new AsyncQueue();
224245
const completedSteps: number[] = [];
@@ -244,7 +265,7 @@ describe('AsyncQueue', () => {
244265
queue.enqueueRetryable(async () => {
245266
doStep(1);
246267
if (completedSteps.length === 1) {
247-
throw new Error('Simulated retryable error');
268+
throw new DOMException('Simulated retryable error');
248269
}
249270
});
250271

@@ -270,7 +291,7 @@ describe('AsyncQueue', () => {
270291
doStep(1);
271292
if (completedSteps.length > 1) {
272293
} else {
273-
throw new Error('Simulated retryable error');
294+
throw new DOMException('Simulated retryable error');
274295
}
275296
});
276297
queue.enqueueRetryable(async () => {

packages/firestore/test/util/node_persistence.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,11 @@ if (process.env.USE_MOCK_PERSISTENCE === 'YES') {
5858

5959
globalAny.Event = Event;
6060
}
61+
62+
if (typeof DOMException === 'undefined') {
63+
// Define DOMException as it is used in tests
64+
class DOMException {
65+
constructor(readonly message: string) {}
66+
}
67+
globalAny.DOMException = DOMException;
68+
}

0 commit comments

Comments
 (0)