-
Notifications
You must be signed in to change notification settings - Fork 934
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
Changes from 5 commits
1119bc3
7e3b47e
6ab88e7
b799fcd
d2ac3b0
c9ee6ea
696afcc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
'IndexedDB transaction failed with environment error: ' + cause | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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(); | ||
} | ||
|
@@ -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)); | ||
}; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
@@ -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); | ||
|
There was a problem hiding this comment.
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.