Skip to content

Commit bf8d206

Browse files
committed
Implemented exponential backoff with query
1 parent 34ad43c commit bf8d206

File tree

6 files changed

+110
-40
lines changed

6 files changed

+110
-40
lines changed

packages/storage/src/implementation/error.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export class StorageError extends FirebaseError {
3535
* added to the end of the message.
3636
* @param message - Error message.
3737
*/
38-
constructor(code: StorageErrorCode, message: string) {
38+
constructor(code: StorageErrorCode, message: string, private status_ = 0) {
3939
super(
4040
prependCode(code),
4141
`Firebase Storage: ${message} (${prependCode(code)})`
@@ -46,6 +46,14 @@ export class StorageError extends FirebaseError {
4646
Object.setPrototypeOf(this, StorageError.prototype);
4747
}
4848

49+
get status(): number {
50+
return this.status_;
51+
}
52+
53+
set status(status: number) {
54+
this.status_ = status;
55+
}
56+
4957
/**
5058
* Compares a StorageErrorCode against this error's code, filtering out the prefix.
5159
*/

packages/storage/src/implementation/request.ts

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { ErrorHandler, RequestHandler, RequestInfo } from './requestinfo';
2626
import { isJustDef } from './type';
2727
import { makeQueryString } from './url';
2828
import { Connection, ErrorCode, Headers, ConnectionType } from './connection';
29+
import { isRetryStatusCode_ } from './utils';
2930

3031
export interface Request<T> {
3132
getPromise(): Promise<T>;
@@ -69,7 +70,8 @@ class NetworkRequest<I extends ConnectionType, O> implements Request<O> {
6970
private errorCallback_: ErrorHandler | null,
7071
private timeout_: number,
7172
private progressCallback_: ((p1: number, p2: number) => void) | null,
72-
private connectionFactory_: () => Connection<I>
73+
private connectionFactory_: () => Connection<I>,
74+
private retry = true
7375
) {
7476
this.promise_ = new Promise((resolve, reject) => {
7577
this.resolve_ = resolve as (value?: O | PromiseLike<O>) => void;
@@ -93,16 +95,15 @@ class NetworkRequest<I extends ConnectionType, O> implements Request<O> {
9395
const connection = this.connectionFactory_();
9496
this.pendingConnection_ = connection;
9597

96-
const progressListener: (progressEvent: ProgressEvent) => void =
97-
progressEvent => {
98-
const loaded = progressEvent.loaded;
99-
const total = progressEvent.lengthComputable
100-
? progressEvent.total
101-
: -1;
102-
if (this.progressCallback_ !== null) {
103-
this.progressCallback_(loaded, total);
104-
}
105-
};
98+
const progressListener: (
99+
progressEvent: ProgressEvent
100+
) => void = progressEvent => {
101+
const loaded = progressEvent.loaded;
102+
const total = progressEvent.lengthComputable ? progressEvent.total : -1;
103+
if (this.progressCallback_ !== null) {
104+
this.progressCallback_(loaded, total);
105+
}
106+
};
106107
if (this.progressCallback_ !== null) {
107108
connection.addUploadProgressListener(progressListener);
108109
}
@@ -118,7 +119,11 @@ class NetworkRequest<I extends ConnectionType, O> implements Request<O> {
118119
this.pendingConnection_ = null;
119120
const hitServer = connection.getErrorCode() === ErrorCode.NO_ERROR;
120121
const status = connection.getStatus();
121-
if (!hitServer || this.isRetryStatusCode_(status)) {
122+
if (
123+
(!hitServer ||
124+
isRetryStatusCode_(status, this.additionalRetryCodes_)) &&
125+
this.retry
126+
) {
122127
const wasCanceled = connection.getErrorCode() === ErrorCode.ABORT;
123128
backoffCallback(
124129
false,
@@ -196,22 +201,6 @@ class NetworkRequest<I extends ConnectionType, O> implements Request<O> {
196201
this.pendingConnection_.abort();
197202
}
198203
}
199-
200-
private isRetryStatusCode_(status: number): boolean {
201-
// The codes for which to retry came from this page:
202-
// https://cloud.google.com/storage/docs/exponential-backoff
203-
const isFiveHundredCode = status >= 500 && status < 600;
204-
const extraRetryCodes = [
205-
// Request Timeout: web server didn't receive full request in time.
206-
408,
207-
// Too Many Requests: you're getting rate-limited, basically.
208-
429
209-
];
210-
const isExtraRetryCode = extraRetryCodes.indexOf(status) !== -1;
211-
const isRequestSpecificRetryCode =
212-
this.additionalRetryCodes_.indexOf(status) !== -1;
213-
return isFiveHundredCode || isExtraRetryCode || isRequestSpecificRetryCode;
214-
}
215204
}
216205

217206
/**
@@ -271,7 +260,8 @@ export function makeRequest<I extends ConnectionType, O>(
271260
authToken: string | null,
272261
appCheckToken: string | null,
273262
requestFactory: () => Connection<I>,
274-
firebaseVersion?: string
263+
firebaseVersion?: string,
264+
retry = true
275265
): Request<O> {
276266
const queryPart = makeQueryString(requestInfo.urlParams);
277267
const url = requestInfo.url + queryPart;
@@ -291,6 +281,7 @@ export function makeRequest<I extends ConnectionType, O>(
291281
requestInfo.errorHandler,
292282
requestInfo.timeout,
293283
requestInfo.progressCallback,
294-
requestFactory
284+
requestFactory,
285+
retry
295286
);
296287
}

packages/storage/src/implementation/requests.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export function sharedErrorHandler(
104104
xhr: Connection<ConnectionType>,
105105
err: StorageError
106106
): StorageError {
107-
let newErr;
107+
let newErr: StorageError;
108108
if (xhr.getStatus() === 401) {
109109
if (
110110
// This exact message string is the only consistent part of the
@@ -126,6 +126,7 @@ export function sharedErrorHandler(
126126
}
127127
}
128128
}
129+
newErr.status = xhr.getStatus();
129130
newErr.serverResponse = err.serverResponse;
130131
return newErr;
131132
}
@@ -534,8 +535,15 @@ export function continueResumableUpload(
534535
}
535536
const startByte = status_.current;
536537
const endByte = startByte + bytesToUpload;
537-
const uploadCommand =
538-
bytesToUpload === bytesLeft ? 'upload, finalize' : 'upload';
538+
let uploadCommand = '';
539+
if (bytesToUpload === 0) {
540+
// TODO(mtewani): Maybe we should extract this out.
541+
uploadCommand = 'finalize';
542+
} else if (bytesLeft === bytesToUpload) {
543+
uploadCommand = 'upload, finalize';
544+
} else {
545+
uploadCommand = 'upload';
546+
}
539547
const headers = {
540548
'X-Goog-Upload-Command': uploadCommand,
541549
'X-Goog-Upload-Offset': `${status_.current}`
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* @license
3+
* Copyright 2022 Google LLC
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
export function isRetryStatusCode_(
19+
status: number,
20+
additionalRetryCodes: number[]
21+
): boolean {
22+
// The codes for which to retry came from this page:
23+
// https://cloud.google.com/storage/docs/exponential-backoff
24+
const isFiveHundredCode = status >= 500 && status < 600;
25+
const extraRetryCodes = [
26+
// Request Timeout: web server didn't receive full request in time.
27+
408,
28+
// Too Many Requests: you're getting rate-limited, basically.
29+
429
30+
];
31+
const isExtraRetryCode = extraRetryCodes.indexOf(status) !== -1;
32+
const isRequestSpecificRetryCode =
33+
additionalRetryCodes.indexOf(status) !== -1;
34+
return isFiveHundredCode || isExtraRetryCode || isRequestSpecificRetryCode;
35+
}

packages/storage/src/service.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,8 @@ export class FirebaseStorageImpl implements FirebaseStorage {
302302
requestInfo: RequestInfo<I, O>,
303303
requestFactory: () => Connection<I>,
304304
authToken: string | null,
305-
appCheckToken: string | null
305+
appCheckToken: string | null,
306+
retry = true
306307
): Request<O> {
307308
if (!this._deleted) {
308309
const request = makeRequest(
@@ -311,7 +312,8 @@ export class FirebaseStorageImpl implements FirebaseStorage {
311312
authToken,
312313
appCheckToken,
313314
requestFactory,
314-
this._firebaseVersion
315+
this._firebaseVersion,
316+
retry
315317
);
316318
this._requests.add(request);
317319
// Request removes itself from set when complete.

packages/storage/src/task.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ import { FbsBlob } from './implementation/blob';
2222
import {
2323
canceled,
2424
StorageErrorCode,
25-
StorageError
25+
StorageError,
26+
retryLimitExceeded
2627
} from './implementation/error';
2728
import {
2829
InternalTaskState,
@@ -53,6 +54,7 @@ import {
5354
} from './implementation/requests';
5455
import { Reference } from './reference';
5556
import { newTextConnection } from './platform/connection';
57+
import { isRetryStatusCode_ } from './implementation/utils';
5658

5759
/**
5860
* Represents a blob being uploaded. Can be used to pause/resume/cancel the
@@ -92,6 +94,15 @@ export class UploadTask {
9294
private _reject?: (p1: StorageError) => void = undefined;
9395
private _promise: Promise<UploadTaskSnapshot>;
9496

97+
// TODO(mtewani): Update these to use predefined constants
98+
private sleepTime = 0;
99+
100+
private maxSleepTime = 10000;
101+
102+
isExponentialBackoffExpired(): boolean {
103+
return this.sleepTime > this.maxSleepTime;
104+
}
105+
95106
/**
96107
* @param ref - The firebaseStorage.Reference object this task came
97108
* from, untyped to avoid cyclic dependencies.
@@ -111,6 +122,17 @@ export class UploadTask {
111122
this._needToFetchStatus = true;
112123
this.completeTransitions_();
113124
} else {
125+
const backoffExpired = this.isExponentialBackoffExpired();
126+
if (isRetryStatusCode_(error.status, [])) {
127+
if (backoffExpired) {
128+
error = retryLimitExceeded();
129+
} else {
130+
this.sleepTime = Math.max(this.sleepTime * 2, 1000);
131+
this._needToFetchStatus = true;
132+
this.completeTransitions_();
133+
return;
134+
}
135+
}
114136
this._error = error;
115137
this._transition(InternalTaskState.ERROR);
116138
}
@@ -163,7 +185,9 @@ export class UploadTask {
163185
// Happens if we miss the metadata on upload completion.
164186
this._fetchMetadata();
165187
} else {
166-
this._continueUpload();
188+
setTimeout(() => {
189+
this._continueUpload();
190+
}, this.sleepTime);
167191
}
168192
}
169193
}
@@ -283,7 +307,8 @@ export class UploadTask {
283307
requestInfo,
284308
newTextConnection,
285309
authToken,
286-
appCheckToken
310+
appCheckToken,
311+
false
287312
);
288313
this._request = uploadRequest;
289314
uploadRequest.getPromise().then((newStatus: ResumableUploadStatus) => {
@@ -344,7 +369,8 @@ export class UploadTask {
344369
requestInfo,
345370
newTextConnection,
346371
authToken,
347-
appCheckToken
372+
appCheckToken,
373+
false
348374
);
349375
this._request = multipartRequest;
350376
multipartRequest.getPromise().then(metadata => {

0 commit comments

Comments
 (0)