Skip to content

Only retry DOMException/DOMError #2919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions packages/firestore/src/local/simple_db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,18 @@ export interface IterateOptions {
reverse?: boolean;
}

/** An error that wraps exceptions that thrown during IndexedDB execution. */
export class IndexedDbTransactionError extends FirestoreError {
name = 'IndexedDbTransactionError';

constructor(cause: Error) {
super(
Code.UNKNOWN,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UNAVAILABLE could be a better code for this given what we know about when this happens.

'IndexedDB transaction failed with environment error: ' + cause
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop "with environment error". "IndexedDB transaction failed" is enough to indicate what's failing and the cause will carry the details.

);
}
}

/**
* Wraps an IDBTransaction and exposes a store() method to get a handle to a
* specific object store.
Expand All @@ -432,7 +444,9 @@ export class SimpleDbTransaction {
};
this.transaction.onabort = () => {
if (transaction.error) {
this.completionDeferred.reject(transaction.error);
this.completionDeferred.reject(
new IndexedDbTransactionError(transaction.error)
);
} else {
this.completionDeferred.resolve();
}
Expand All @@ -441,7 +455,7 @@ export class SimpleDbTransaction {
const error = checkForAndReportiOSError(
(event.target as IDBRequest).error!
);
this.completionDeferred.reject(error);
this.completionDeferred.reject(new IndexedDbTransactionError(error));
};
}

Expand Down
17 changes: 12 additions & 5 deletions packages/firestore/src/util/async_queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { logDebug, logError } from './log';
import { CancelablePromise, Deferred } from './promise';
import { ExponentialBackoff } from '../remote/backoff';
import { PlatformSupport } from '../platform/platform';
import { IndexedDbTransactionError } from '../local/simple_db';

const LOG_TAG = 'AsyncQueue';

Expand Down Expand Up @@ -318,9 +319,10 @@ export class AsyncQueue {
/**
* Enqueue a retryable operation.
*
* A retryable operation is rescheduled with backoff if it fails with any
* exception. All retryable operations are executed in order and only run
* if all prior operations were retried successfully.
* A retryable operation is rescheduled with backoff if it fails with a
* SimpleDbTransactionError (the error type used by SimpleDb). All
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is actually IndexedDbTransactionError.

* retryable operations are executed in order and only run if all prior
* operations were retried successfully.
*/
enqueueRetryable(op: () => Promise<void>): void {
this.verifyNotFailed();
Expand All @@ -337,8 +339,13 @@ export class AsyncQueue {
deferred.resolve();
this.backoff.reset();
} catch (e) {
logDebug(LOG_TAG, 'Retryable operation failed: ' + e.message);
this.backoff.backoffAndRun(retryingOp);
if (e.name === 'IndexedDbTransactionError') {
logDebug(LOG_TAG, 'Operation failed with retryable error: ' + e);
this.backoff.backoffAndRun(retryingOp);
} else {
deferred.resolve();
throw e; // Failure will be handled by AsyncQueue
}
}
};
this.enqueueAndForget(retryingOp);
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/unit/specs/recovery_spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describeSpec(
// Client 1 has received the WebStorage notification that the write
// has been acknowledged, but failed to process the change. Hence,
// we did not get a user callback. We schedule the first retry and
// make sure that it also does not get processed until
// make sure that it also does not get processed until
// `recoverDatabase` is called.
.runTimer(TimerId.AsyncQueueRetry)
.recoverDatabase()
Expand Down
5 changes: 4 additions & 1 deletion packages/firestore/test/unit/specs/spec_test_components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { TargetCache } from '../../../src/local/target_cache';
import { RemoteDocumentCache } from '../../../src/local/remote_document_cache';
import { IndexManager } from '../../../src/local/index_manager';
import { PersistencePromise } from '../../../src/local/persistence_promise';
import { IndexedDbTransactionError } from '../../../src/local/simple_db';
import { debugAssert } from '../../../src/util/assert';
import {
MemoryEagerDelegate,
Expand Down Expand Up @@ -113,7 +114,9 @@ export class MockPersistence implements Persistence {
) => PersistencePromise<T>
): Promise<T> {
if (this.injectFailures) {
return Promise.reject(new Error('Injected Failure'));
return Promise.reject(
new IndexedDbTransactionError(new Error('Simulated retryable error'))
);
} else {
return this.delegate.runTransaction(action, mode, transactionOperation);
}
Expand Down
36 changes: 32 additions & 4 deletions packages/firestore/test/unit/util/async_queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@
* limitations under the License.
*/

import { expect } from 'chai';
import * as chaiAsPromised from 'chai-as-promised';

import { expect, use } from 'chai';
import { AsyncQueue, TimerId } from '../../../src/util/async_queue';
import { Code } from '../../../src/util/error';
import { getLogLevel, setLogLevel, LogLevel } from '../../../src/util/log';
import { Deferred, Rejecter, Resolver } from '../../../src/util/promise';
import { fail } from '../../../src/util/assert';
import { IndexedDbTransactionError } from '../../../src/local/simple_db';

use(chaiAsPromised);

describe('AsyncQueue', () => {
// We reuse these TimerIds for generic testing.
Expand Down Expand Up @@ -212,13 +218,31 @@ describe('AsyncQueue', () => {
queue.enqueueRetryable(async () => {
doStep(1);
if (completedSteps.length === 1) {
throw new Error('Simulated retryable error');
throw new IndexedDbTransactionError(
new Error('Simulated retryable error')
);
}
});
await queue.runDelayedOperationsEarly(TimerId.AsyncQueueRetry);
expect(completedSteps).to.deep.equal([1, 1]);
});

it("Doesn't retry internal exceptions", async () => {
const queue = new AsyncQueue();
// We use a deferred Promise as retryable operations are scheduled only
// when Promise chains are resolved, which can happen after the
// `queue.enqueue()` call below.
const deferred = new Deferred<void>();
queue.enqueueRetryable(async () => {
deferred.resolve();
throw fail('Simulated test failure');
});
await deferred.promise;
await expect(
queue.enqueue(() => Promise.resolve())
).to.eventually.be.rejectedWith('Simulated test failure');
});

it('Schedules first retryable attempt with no delay', async () => {
const queue = new AsyncQueue();
const completedSteps: number[] = [];
Expand All @@ -244,7 +268,9 @@ describe('AsyncQueue', () => {
queue.enqueueRetryable(async () => {
doStep(1);
if (completedSteps.length === 1) {
throw new Error('Simulated retryable error');
throw new IndexedDbTransactionError(
new Error('Simulated retryable error')
);
}
});

Expand All @@ -270,7 +296,9 @@ describe('AsyncQueue', () => {
doStep(1);
if (completedSteps.length > 1) {
} else {
throw new Error('Simulated retryable error');
throw new IndexedDbTransactionError(
new Error('Simulated retryable error')
);
}
});
queue.enqueueRetryable(async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/util/node_persistence.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* @license
* Copyright 2018 Google Inc.
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down