-
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 4 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 |
---|---|---|
|
@@ -318,9 +318,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 | ||
* DOMException or a DOMError (the error types used by IndexedDB). All | ||
* 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 +338,19 @@ export class AsyncQueue { | |
deferred.resolve(); | ||
this.backoff.reset(); | ||
} catch (e) { | ||
logDebug(LOG_TAG, 'Retryable operation failed: ' + e.message); | ||
this.backoff.backoffAndRun(retryingOp); | ||
// We only retry errors that match the ones thrown by IndexedDB (see | ||
// https://developer.mozilla.org/en-US/docs/Web/API/IDBTransaction/error) | ||
if ( | ||
(typeof DOMException !== 'undefined' && | ||
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.
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. Done. I am now wrapping exceptions thrown by |
||
e instanceof DOMException) || | ||
(typeof DOMError !== 'undefined' && e instanceof DOMError) | ||
) { | ||
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.
While this seems wrong, this is needed to pull in
DOMException
. SinceUSE_MOCK_PERSISTENCE
is not set, there are no other side effects. I chose not to rename this file since it is used in our test launchers.