Skip to content

Commit

Permalink
feat: customize retry behavior implementation (#1474) (#1493)
Browse files Browse the repository at this point in the history 
* feat: customize retry behavior implementation (#1474)

* feat: customize retry behavior implementation

* 🦉 Updates from OwlBot

* fixed !=

* 🦉 Updates from OwlBot

* updated names to match gogle gax

* 🦉 Updates from OwlBot

* refactored retryOptions into its own config

* 🦉 Updates from OwlBot

* fixed linting error

* 🦉 Updates from OwlBot

* added retry delay explanation

* 🦉 Updates from OwlBot

* refactored constants

* 🦉 Updates from OwlBot

* removed const assignment

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* feat: implemented retry function (#1476)

* implemented retry function

* 🦉 Updates from OwlBot

* fixed import

* 🦉 Updates from OwlBot

* resolved merge conflict

* passed retry function to common

* 🦉 Updates from OwlBot

* refactored code to retryableErrFn

* removed unused import

* fixed typo

* fixed typo

* fixed failing tests

* made retryableErrorFn configurable

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* wrote unit tests

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* changed reason check to be less brittle

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* depend on common 3.7.0 for retry changes

* feat: remove gaxios dependency (#1503)

* feat: remove gaxios dependency

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* moved callbackFunction inline

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* assigned types to any

* changed any to string

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* feat: applied customization to multipart filesave (#1504)

* feat: applied customization to multipart filesave

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* fixed failing tests

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* removed unused import

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* Update storage.ts

added comment next to EAI_AGAIN

* feat: pass retryOptions to gcs-resumable-upload (#1506)

* feat: pass retryOptions to gcs-resumable-upload

* linter fixes

* additional test assertions for retryOptions

* add additional asserts to resumable operation tests

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com>
  • Loading branch information
@shaffeeullah @gcf-owl-bot @ddelgrosso1
3 people committed Jul 21, 2021
1 parent 742567b commit 49008e3
Show file tree
Hide file tree
Showing 5 changed files with 456 additions and 45 deletions.
5 changes: 2 additions & 3 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"precompile": "gts clean"
},
"dependencies": {
"@google-cloud/common": "^3.6.0",
"@google-cloud/common": "^3.7.0",
"@google-cloud/paginator": "^3.0.0",
"@google-cloud/promisify": "^2.0.0",
"arrify": "^2.0.0",
Expand All @@ -59,8 +59,7 @@
"date-and-time": "^1.0.0",
"duplexify": "^4.0.0",
"extend": "^3.0.2",
"gaxios": "^4.0.0",
"gcs-resumable-upload": "^3.1.4",
"gcs-resumable-upload": "^3.3.0",
"get-stream": "^6.0.0",
"hash-stream-validation": "^0.2.2",
"mime": "^2.2.0",
Expand Down
49 changes: 35 additions & 14 deletions src/file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ import {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const duplexify: DuplexifyConstructor = require('duplexify');
import {normalize, objectKeyToLowercase, unicodeJSONStringify} from './util';
import {GaxiosError, Headers, request as gaxiosRequest} from 'gaxios';
import retry = require('async-retry');

export type GetExpirationDateResponse = [Date];
Expand Down Expand Up @@ -1254,6 +1253,10 @@ class File extends ServiceObject<File> {
query.userProject = options.userProject;
}

interface Headers {
[index: string]: string;
}

const headers = {
'Accept-Encoding': 'gzip',
'Cache-Control': 'no-store',
Expand Down Expand Up @@ -1560,6 +1563,7 @@ class File extends ServiceObject<File> {
private: options.private,
public: options.public,
userProject: options.userProject || this.userProject,
retryOptions: this.storage.retryOptions,
},
callback!
);
Expand Down Expand Up @@ -1962,6 +1966,7 @@ class File extends ServiceObject<File> {
bucket: this.bucket.name,
file: this.name,
generation: this.generation,
retryOptions: this.storage.retryOptions,
});
uploadStream.deleteConfig();
}
Expand Down Expand Up @@ -2993,18 +2998,26 @@ class File extends ServiceObject<File> {
*/

isPublic(callback?: IsPublicCallback): Promise<IsPublicResponse> | void {
gaxiosRequest({
method: 'HEAD',
url: `http://${
this.bucket.name
}.storage.googleapis.com/${encodeURIComponent(this.name)}`,
}).then(
() => callback!(null, true),
(err: GaxiosError) => {
if (err.code === '403') {
callback!(null, false);
util.makeRequest(
{
method: 'HEAD',
uri: `http://${
this.bucket.name
}.storage.googleapis.com/${encodeURIComponent(this.name)}`,
},
{
retryOptions: this.storage.retryOptions,
},
(err: Error | ApiError | null) => {
if (err) {
const apiError = err as ApiError;
if (apiError.code === 403) {
callback!(null, false);
} else {
callback!(err);
}
} else {
callback!(err);
callback!(null, true);
}
}
);
Expand Down Expand Up @@ -3611,7 +3624,11 @@ class File extends ServiceObject<File> {
await new Promise<void>((resolve, reject) => {
const writable = this.createWriteStream(options)
.on('error', err => {
if (isMultipart && util.shouldRetryRequest(err)) {
if (
isMultipart &&
this.storage.retryOptions.autoRetry &&
this.storage.retryOptions.retryableErrorFn!(err)
) {
return reject(err);
} else {
return bail(err);
Expand All @@ -3627,7 +3644,10 @@ class File extends ServiceObject<File> {
});
},
{
retries: 3,
retries: this.storage.retryOptions.maxRetries,
factor: this.storage.retryOptions.retryDelayMultiplier,
maxTimeout: this.storage.retryOptions.maxRetryDelay! * 1000, //convert to milliseconds
maxRetryTime: this.storage.retryOptions.totalTimeout! * 1000, //convert to milliseconds
}
);
if (!callback) {
Expand Down Expand Up @@ -3793,6 +3813,7 @@ class File extends ServiceObject<File> {
public: options.public,
uri: options.uri,
userProject: options.userProject || this.userProject,
retryOptions: this.storage.retryOptions,
});

uploadStream
Expand Down
157 changes: 150 additions & 7 deletions src/storage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {Metadata, Service, ServiceOptions} from '@google-cloud/common';
import {
ApiError,
Metadata,
Service,
ServiceOptions,
} from '@google-cloud/common';
import {paginator} from '@google-cloud/paginator';
import {promisifyAll} from '@google-cloud/promisify';

import arrify = require('arrify');
import {Readable} from 'stream';

Expand Down Expand Up @@ -45,9 +49,19 @@ export interface CreateBucketQuery {
userProject: string;
}

export interface StorageOptions extends ServiceOptions {
export interface RetryOptions {
retryDelayMultiplier?: number;
totalTimeout?: number;
maxRetryDelay?: number;
autoRetry?: boolean;
maxRetries?: number;
retryableErrorFn?: (err: ApiError) => boolean;
}

export interface StorageOptions extends ServiceOptions {
retryOptions?: RetryOptions;
autoRetry?: boolean; //Deprecated. Use retryOptions instead.
maxRetries?: number; //Deprecated. Use retryOptions instead.
/**
* **This option is deprecated.**
* @todo Remove in next major release.
Expand Down Expand Up @@ -163,6 +177,77 @@ export type GetHmacKeysResponse = [HmacKey[]];

export const PROTOCOL_REGEX = /^(\w*):\/\//;

/**
* Default behavior: Automatically retry retriable server errors.
*
* @const {boolean}
* @private
*/
const AUTO_RETRY_DEFAULT = true;

/**
* Default behavior: Only attempt to retry retriable errors 3 times.
*
* @const {number}
* @private
*/
const MAX_RETRY_DEFAULT = 3;

/**
* Default behavior: Wait twice as long as previous retry before retrying.
*
* @const {number}
* @private
*/
const RETRY_DELAY_MULTIPLIER_DEFAULT = 2;

/**
* Default behavior: If the operation doesn't succeed after 600 seconds,
* stop retrying.
*
* @const {number}
* @private
*/
const TOTAL_TIMEOUT_DEFAULT = 600;

/**
* Default behavior: Wait no more than 64 seconds between retries.
*
* @const {number}
* @private
*/
const MAX_RETRY_DELAY_DEFAULT = 64;

/**
* Returns true if the API request should be retried, given the error that was
* given the first time the request was attempted.
* @const
* @private
* @param {error} err - The API error to check if it is appropriate to retry.
* @return {boolean} True if the API request should be retried, false otherwise.
*/
const RETRYABLE_ERR_FN_DEFAULT = function (err?: ApiError) {
if (err) {
if ([408, 429, 500, 502, 503, 504].indexOf(err.code!) !== -1) {
return true;
}

if (err.errors) {
for (const e of err.errors) {
const reason = e.reason?.toLowerCase();
if (
(reason && reason.includes('eai_again')) || //DNS lookup error
reason === 'connection reset by peer' ||
reason === 'unexpected connection closure'
) {
return true;
}
}
}
}
return false;
};

/*! Developer Documentation
*
* Invoke this method to create a new Storage object bound with pre-determined
Expand Down Expand Up @@ -350,6 +435,8 @@ export class Storage extends Service {
getBucketsStream: () => Readable;
getHmacKeysStream: () => Readable;

retryOptions: RetryOptions;

/**
* @typedef {object} StorageOptions
* @property {string} [projectId] The project ID from the Google Developer's
Expand All @@ -368,11 +455,26 @@ export class Storage extends Service {
* @property {object} [credentials] Credentials object.
* @property {string} [credentials.client_email]
* @property {string} [credentials.private_key]
* @property {boolean} [autoRetry=true] Automatically retry requests if the
* @property {object} [retryOptions] Options for customizing retries. Retriable server errors
* will be retried with exponential delay between them dictated by the formula
* max(maxRetryDelay, retryDelayMultiplier*retryNumber) until maxRetries or totalTimeout
* has been reached. Retries will only happen if autoRetry is set to true.
* @property {boolean} [retryOptions.autoRetry=true] Automatically retry requests if the
* response is related to rate limits or certain intermittent server
* errors. We will exponentially backoff subsequent requests by default.
* @property {number} [maxRetries=3] Maximum number of automatic retries
* @property {number} [retryOptions.retryDelayMultiplier = 2] the multiplier by which to
* increase the delay time between the completion of failed requests, and the
* initiation of the subsequent retrying request.
* @property {number} [retryOptions.totalTimeout = 600] The total time, starting from
* when the initial request is sent, after which an error will
* be returned, regardless of the retrying attempts made meanwhile.
* @property {number} [retryOptions.maxRetryDelay = 64] The maximum delay time between requests.
* When this value is reached, ``retryDelayMultiplier`` will no longer be used to
* increase delay time.
* @property {number} [retryOptions.maxRetries=3] Maximum number of automatic retries
* attempted before returning the error.
* @property {function} [retryOptions.retryableErrorFn] Function that returns true if a given
* error should be retried and false otherwise.
* @property {string} [userAgent] The value to be prepended to the User-Agent
* header in API requests.
*/
Expand Down Expand Up @@ -413,10 +515,49 @@ export class Storage extends Service {
// Note: EMULATOR_HOST is an experimental configuration variable. Use apiEndpoint instead.
const baseUrl = EMULATOR_HOST || `${options.apiEndpoint}/storage/v1`;

let autoRetryValue = AUTO_RETRY_DEFAULT;
if (
options.autoRetry !== undefined &&
options.retryOptions?.autoRetry !== undefined
) {
throw new ApiError(
'autoRetry is deprecated. Use retryOptions.autoRetry instead.'
);
} else if (options.autoRetry !== undefined) {
autoRetryValue = options.autoRetry;
} else if (options.retryOptions?.autoRetry !== undefined) {
autoRetryValue = options.retryOptions.autoRetry;
}

let maxRetryValue = MAX_RETRY_DEFAULT;
if (options.maxRetries && options.retryOptions?.maxRetries) {
throw new ApiError(
'maxRetries is deprecated. Use retryOptions.maxRetries instead.'
);
} else if (options.maxRetries) {
maxRetryValue = options.maxRetries;
} else if (options.retryOptions?.maxRetries) {
maxRetryValue = options.retryOptions.maxRetries;
}

const config = {
apiEndpoint: options.apiEndpoint!,
autoRetry: options.autoRetry,
maxRetries: options.maxRetries,
retryOptions: {
autoRetry: autoRetryValue,
maxRetries: maxRetryValue,
retryDelayMultiplier: options.retryOptions?.retryDelayMultiplier
? options.retryOptions?.retryDelayMultiplier
: RETRY_DELAY_MULTIPLIER_DEFAULT,
totalTimeout: options.retryOptions?.totalTimeout
? options.retryOptions?.totalTimeout
: TOTAL_TIMEOUT_DEFAULT,
maxRetryDelay: options.retryOptions?.maxRetryDelay
? options.retryOptions?.maxRetryDelay
: MAX_RETRY_DELAY_DEFAULT,
retryableErrorFn: options.retryOptions?.retryableErrorFn
? options.retryOptions?.retryableErrorFn
: RETRYABLE_ERR_FN_DEFAULT,
},
baseUrl,
customEndpoint,
projectIdRequired: false,
Expand All @@ -438,6 +579,8 @@ export class Storage extends Service {
*/
this.acl = Storage.acl;

this.retryOptions = config.retryOptions;

this.getBucketsStream = paginator.streamify('getBuckets');
this.getHmacKeysStream = paginator.streamify('getHmacKeys');
}
Expand Down
Loading

0 comments on commit 49008e3

Please sign in to comment.