diff --git a/.github/workflows/conformance-test.yaml b/.github/workflows/conformance-test.yaml index cb9912c9b166..344510e2962d 100644 --- a/.github/workflows/conformance-test.yaml +++ b/.github/workflows/conformance-test.yaml @@ -15,7 +15,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v6 with: - node-version: 14 + node-version: 18 - run: node --version - run: cd handwritten/storage && npm install - run: cd handwritten/storage && npm run conformance-test diff --git a/handwritten/storage/src/file.ts b/handwritten/storage/src/file.ts index 850a0991f9e3..72ce0296d669 100644 --- a/handwritten/storage/src/file.ts +++ b/handwritten/storage/src/file.ts @@ -1405,7 +1405,12 @@ class File extends ServiceObject { } if (newFile.encryptionKey !== undefined) { - this.setEncryptionKey(newFile.encryptionKey!); + headers.set('x-goog-encryption-algorithm', 'AES256'); + headers.set('x-goog-encryption-key', newFile.encryptionKeyBase64 || ''); + headers.set( + 'x-goog-encryption-key-sha256', + newFile.encryptionKeyHash || '', + ); } else if (options.destinationKmsKeyName !== undefined) { query.destinationKmsKeyName = options.destinationKmsKeyName; delete options.destinationKmsKeyName; @@ -1639,6 +1644,8 @@ class File extends ServiceObject { } const headers = response.headers; + const isStoredCompressed = + headers.get('x-goog-stored-content-encoding') === 'gzip'; const isCompressed = headers.get('content-encoding') === 'gzip'; const hashes: {crc32c?: string; md5?: string} = {}; @@ -1652,7 +1659,7 @@ class File extends ServiceObject { const transformStreams: Transform[] = []; - if (shouldRunValidation) { + if (shouldRunValidation && !isStoredCompressed) { // The x-goog-hash header should be set with a crc32c and md5 hash. // ex: headers.set('x-goog-hash', 'crc32c=xxxx,md5=xxxx') if (typeof headers.get('x-goog-hash') === 'string') { @@ -1721,6 +1728,7 @@ class File extends ServiceObject { const headers = { 'Accept-Encoding': 'gzip', 'Cache-Control': 'no-store', + ...(this.encryptionKeyHeaders || {}), } as Headers; if (rangeRequest) { @@ -1735,7 +1743,9 @@ class File extends ServiceObject { headers, queryParameters: query as unknown as StorageQueryParameters, responseType: 'stream', - }; + decompress: options.decompress, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; if (options[GCCL_GCS_CMD_KEY]) { reqOpts[GCCL_GCS_CMD_KEY] = options[GCCL_GCS_CMD_KEY]; @@ -2426,6 +2436,18 @@ class File extends ServiceObject { } } + get encryptionKeyHeaders(): Record | undefined { + if (!this.encryptionKey) { + return undefined; + } + + return { + 'x-goog-encryption-algorithm': 'AES256', + 'x-goog-encryption-key': this.encryptionKey.toString('base64'), + 'x-goog-encryption-key-sha256': this.encryptionKeyHash || '', + }; + } + /** * The Storage API allows you to use a custom key for server-side encryption. * @@ -4562,6 +4584,14 @@ class File extends ServiceObject { }, ]; + const headers: Record = {}; + if (this.encryptionKey) { + headers['x-goog-encryption-algorithm'] = 'AES256'; + headers['x-goog-encryption-key'] = this.encryptionKeyBase64!; + headers['x-goog-encryption-key-sha256'] = this.encryptionKeyHash!; + } + reqOpts.headers = headers; + this.storageTransport .makeRequest(reqOpts as StorageRequestOptions, (err, body, resp) => { if (err) { diff --git a/handwritten/storage/src/nodejs-common/service-object.ts b/handwritten/storage/src/nodejs-common/service-object.ts index 80ed207764d8..f37e41a47b1c 100644 --- a/handwritten/storage/src/nodejs-common/service-object.ts +++ b/handwritten/storage/src/nodejs-common/service-object.ts @@ -444,6 +444,20 @@ class ServiceObject extends EventEmitter { url = `${this.parent.baseUrl}/${this.parent.id}${url}`; } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const encryptionHeaders = (this as any).encryptionKeyHeaders || {}; + + const headers = { + ...encryptionHeaders, + ...methodConfig.reqOpts?.headers, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...(options as any).headers, + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const query = {...options} as any; + delete query.headers; + this.storageTransport .makeRequest( { @@ -451,9 +465,10 @@ class ServiceObject extends EventEmitter { responseType: 'json', url, ...methodConfig.reqOpts, + headers, queryParameters: { ...methodConfig.reqOpts?.queryParameters, - ...options, + ...query, }, }, (err, data, resp) => { diff --git a/handwritten/storage/src/nodejs-common/service.ts b/handwritten/storage/src/nodejs-common/service.ts deleted file mode 100644 index 6e2a6cb90789..000000000000 --- a/handwritten/storage/src/nodejs-common/service.ts +++ /dev/null @@ -1,316 +0,0 @@ -/*! - * Copyright 2022 Google LLC. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { - AuthClient, - DEFAULT_UNIVERSE, - GoogleAuth, - GoogleAuthOptions, -} from 'google-auth-library'; -import * as r from 'teeny-request'; -import * as uuid from 'uuid'; - -import {Interceptor} from './service-object.js'; -import { - BodyResponseCallback, - DecorateRequestOptions, - GCCL_GCS_CMD_KEY, - MakeAuthenticatedRequest, - PackageJson, - util, -} from './util.js'; -import { - getRuntimeTrackingString, - getUserAgentString, - getModuleFormat, -} from '../util.js'; - -export const DEFAULT_PROJECT_ID_TOKEN = '{{projectId}}'; - -export interface StreamRequestOptions extends DecorateRequestOptions { - shouldReturnStream: true; -} - -export interface ServiceConfig { - /** - * The base URL to make API requests to. - */ - baseUrl: string; - - /** - * The API Endpoint to use when connecting to the service. - * Example: storage.googleapis.com - */ - apiEndpoint: string; - - /** - * The scopes required for the request. - */ - scopes: string[]; - - projectIdRequired?: boolean; - packageJson: PackageJson; - - /** - * Reuse an existing `AuthClient` or `GoogleAuth` client instead of creating a new one. - */ - authClient?: AuthClient | GoogleAuth; - - /** - * Set to true if the endpoint is a custom URL - */ - customEndpoint?: boolean; - - /** - * Controls whether or not to use authentication when using a custom endpoint. - */ - useAuthWithCustomEndpoint?: boolean; -} - -export interface ServiceOptions extends Omit { - authClient?: AuthClient | GoogleAuth; - interceptors_?: Interceptor[]; - email?: string; - token?: string; - timeout?: number; // http.request.options.timeout - userAgent?: string; - useAuthWithCustomEndpoint?: boolean; -} - -export class Service { - baseUrl: string; - private globalInterceptors: Interceptor[]; - interceptors: Interceptor[]; - private packageJson: PackageJson; - projectId: string; - private projectIdRequired: boolean; - providedUserAgent?: string; - makeAuthenticatedRequest: MakeAuthenticatedRequest; - authClient: GoogleAuth; - apiEndpoint: string; - timeout?: number; - universeDomain: string; - customEndpoint: boolean; - useAuthWithCustomEndpoint?: boolean; - - /** - * Service is a base class, meant to be inherited from by a "service," like - * BigQuery or Storage. - * - * This handles making authenticated requests by exposing a `makeReq_` - * function. - * - * @constructor - * @alias module:common/service - * - * @param {object} config - Configuration object. - * @param {string} config.baseUrl - The base URL to make API requests to. - * @param {string[]} config.scopes - The scopes required for the request. - * @param {object=} options - [Configuration object](#/docs). - */ - constructor(config: ServiceConfig, options: ServiceOptions = {}) { - this.baseUrl = config.baseUrl; - this.apiEndpoint = config.apiEndpoint; - this.timeout = options.timeout; - this.globalInterceptors = Array.isArray(options.interceptors_) - ? options.interceptors_ - : []; - this.interceptors = []; - this.packageJson = config.packageJson; - this.projectId = options.projectId || DEFAULT_PROJECT_ID_TOKEN; - this.projectIdRequired = config.projectIdRequired !== false; - this.providedUserAgent = options.userAgent; - this.universeDomain = options.universeDomain || DEFAULT_UNIVERSE; - this.customEndpoint = config.customEndpoint || false; - this.useAuthWithCustomEndpoint = config.useAuthWithCustomEndpoint; - - this.makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory({ - ...config, - projectIdRequired: this.projectIdRequired, - projectId: this.projectId, - authClient: options.authClient || config.authClient, - credentials: options.credentials, - keyFile: options.keyFilename, - email: options.email, - clientOptions: { - universeDomain: options.universeDomain, - ...options.clientOptions, - }, - }); - this.authClient = this.makeAuthenticatedRequest.authClient; - - const isCloudFunctionEnv = !!process.env.FUNCTION_NAME; - - if (isCloudFunctionEnv) { - this.interceptors.push({ - request(reqOpts: DecorateRequestOptions) { - reqOpts.forever = false; - return reqOpts; - }, - }); - } - } - - /** - * Return the user's custom request interceptors. - */ - getRequestInterceptors(): Function[] { - // Interceptors should be returned in the order they were assigned. - return ([] as Interceptor[]).slice - .call(this.globalInterceptors) - .concat(this.interceptors) - .filter(interceptor => typeof interceptor.request === 'function') - .map(interceptor => interceptor.request); - } - - /** - * Get and update the Service's project ID. - * - * @param {function} callback - The callback function. - */ - getProjectId(): Promise; - getProjectId(callback: (err: Error | null, projectId?: string) => void): void; - getProjectId( - callback?: (err: Error | null, projectId?: string) => void, - ): Promise | void { - if (!callback) { - return this.getProjectIdAsync(); - } - this.getProjectIdAsync().then(p => callback(null, p), callback); - } - - protected async getProjectIdAsync(): Promise { - const projectId = await this.authClient.getProjectId(); - if (this.projectId === DEFAULT_PROJECT_ID_TOKEN && projectId) { - this.projectId = projectId; - } - return this.projectId; - } - - /** - * Make an authenticated API request. - * - * @private - * - * @param {object} reqOpts - Request options that are passed to `request`. - * @param {string} reqOpts.uri - A URI relative to the baseUrl. - * @param {function} callback - The callback function passed to `request`. - */ - private request_(reqOpts: StreamRequestOptions): r.Request; - private request_( - reqOpts: DecorateRequestOptions, - callback: BodyResponseCallback, - ): void; - private request_( - reqOpts: DecorateRequestOptions | StreamRequestOptions, - callback?: BodyResponseCallback, - ): void | r.Request { - reqOpts = {...reqOpts, timeout: this.timeout}; - const isAbsoluteUrl = reqOpts.uri.indexOf('http') === 0; - const uriComponents = [this.baseUrl]; - - if (this.projectIdRequired) { - if (reqOpts.projectId) { - uriComponents.push('projects'); - uriComponents.push(reqOpts.projectId); - } else { - uriComponents.push('projects'); - uriComponents.push(this.projectId); - } - } - - uriComponents.push(reqOpts.uri); - - if (isAbsoluteUrl) { - uriComponents.splice(0, uriComponents.indexOf(reqOpts.uri)); - } - - reqOpts.uri = uriComponents - .map(uriComponent => { - const trimSlashesRegex = /^\/*|\/*$/g; - return uriComponent.replace(trimSlashesRegex, ''); - }) - .join('/') - // Some URIs have colon separators. - // Bad: https://.../projects/:list - // Good: https://.../projects:list - .replace(/\/:/g, ':'); - - const requestInterceptors = this.getRequestInterceptors(); - const interceptorArray = Array.isArray(reqOpts.interceptors_) - ? reqOpts.interceptors_ - : []; - interceptorArray.forEach(interceptor => { - if (typeof interceptor.request === 'function') { - requestInterceptors.push(interceptor.request); - } - }); - - requestInterceptors.forEach(requestInterceptor => { - reqOpts = requestInterceptor(reqOpts); - }); - - delete reqOpts.interceptors_; - - const pkg = this.packageJson; - let userAgent = getUserAgentString(); - if (this.providedUserAgent) { - userAgent = `${this.providedUserAgent} ${userAgent}`; - } - reqOpts.headers = { - ...reqOpts.headers, - 'User-Agent': userAgent, - 'x-goog-api-client': `${getRuntimeTrackingString()} gccl/${ - pkg.version - }-${getModuleFormat()} gccl-invocation-id/${uuid.v4()}`, - }; - - if (reqOpts[GCCL_GCS_CMD_KEY]) { - reqOpts.headers['x-goog-api-client'] += - ` gccl-gcs-cmd/${reqOpts[GCCL_GCS_CMD_KEY]}`; - } - - if (reqOpts.shouldReturnStream) { - return this.makeAuthenticatedRequest(reqOpts) as {} as r.Request; - } else { - this.makeAuthenticatedRequest(reqOpts, callback); - } - } - - /** - * Make an authenticated API request. - * - * @param {object} reqOpts - Request options that are passed to `request`. - * @param {string} reqOpts.uri - A URI relative to the baseUrl. - * @param {function} callback - The callback function passed to `request`. - */ - request( - reqOpts: DecorateRequestOptions, - callback: BodyResponseCallback, - ): void { - Service.prototype.request_.call(this, reqOpts, callback); - } - - /** - * Make an authenticated API request. - * - * @param {object} reqOpts - Request options that are passed to `request`. - * @param {string} reqOpts.uri - A URI relative to the baseUrl. - */ - requestStream(reqOpts: DecorateRequestOptions): r.Request { - const opts = {...reqOpts, shouldReturnStream: true}; - return (Service.prototype.request_ as Function).call(this, opts); - } -} diff --git a/handwritten/storage/src/storage-transport.ts b/handwritten/storage/src/storage-transport.ts index 43070a73ff5e..d62f9cdac8d5 100644 --- a/handwritten/storage/src/storage-transport.ts +++ b/handwritten/storage/src/storage-transport.ts @@ -25,13 +25,13 @@ import { getModuleFormat, getRuntimeTrackingString, getUserAgentString, -} from './util'; +} from './util.js'; import {randomUUID} from 'crypto'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore import {getPackageJSON} from './package-json-helper.cjs'; -import {GCCL_GCS_CMD_KEY} from './nodejs-common/util'; -import {RetryOptions} from './storage'; +import {GCCL_GCS_CMD_KEY} from './nodejs-common/util.js'; +import {RETRYABLE_ERR_FN_DEFAULT, RetryOptions} from './storage.js'; export interface StandardStorageQueryParams { alt?: 'json' | 'media'; @@ -87,7 +87,6 @@ export interface StorageTransportCallback { fullResponse?: GaxiosResponse, ): void; } -let projectId: string; export class StorageTransport { authClient: GoogleAuth; @@ -113,7 +112,11 @@ export class StorageTransport { } this.providedUserAgent = options.userAgent; this.packageJson = getPackageJSON(); - this.retryOptions = options.retryOptions; + this.retryOptions = { + ...options.retryOptions, + retryableErrorFn: + options.retryOptions?.retryableErrorFn || RETRYABLE_ERR_FN_DEFAULT, + }; this.baseUrl = options.baseUrl; this.timeout = options.timeout; this.projectId = options.projectId; @@ -124,76 +127,101 @@ export class StorageTransport { reqOpts: StorageRequestOptions, callback?: StorageTransportCallback, ): Promise { - const headers = this.#buildRequestHeaders(reqOpts.headers); - if (reqOpts[GCCL_GCS_CMD_KEY]) { - headers.set( - 'x-goog-api-client', - `${headers.get('x-goog-api-client')} gccl-gcs-cmd/${reqOpts[GCCL_GCS_CMD_KEY]}`, - ); + // Project ID Resolution + if (!this.projectId) { + this.projectId = + reqOpts.projectId || (await this.authClient.getProjectId()); } + + // Header Construction + const headers = this.#prepareHeaders(reqOpts); + + // Interceptor Management + this.gaxiosInstance.interceptors.request.clear(); if (reqOpts.interceptors) { - this.gaxiosInstance.interceptors.request.clear(); for (const inter of reqOpts.interceptors) { this.gaxiosInstance.interceptors.request.add(inter); } } - try { - const getProjectId = async () => { - if (reqOpts.projectId) return reqOpts.projectId; - projectId = await this.authClient.getProjectId(); - return projectId; - }; - const _projectId = await getProjectId(); - if (_projectId) { - projectId = _projectId; - this.projectId = projectId; - } + const urlString = reqOpts.url?.toString() || ''; + const isAbsolute = this.#isValidUrl(urlString); + + // Determine the base URL for the request + const requestUrl = isAbsolute + ? urlString + : new URL(urlString, this.baseUrl).toString(); + try { const requestPromise = this.authClient.request({ + adapter: this.gaxiosInstance.request.bind(this.gaxiosInstance), retryConfig: { retry: this.retryOptions.maxRetries, noResponseRetries: this.retryOptions.maxRetries, maxRetryDelay: this.retryOptions.maxRetryDelay, retryDelayMultiplier: this.retryOptions.retryDelayMultiplier, - shouldRetry: this.retryOptions.retryableErrorFn, totalTimeout: this.retryOptions.totalTimeout, + shouldRetry: err => !!this.retryOptions.retryableErrorFn?.(err), }, ...reqOpts, + params: reqOpts.queryParameters, + paramsSerializer: this.#paramsSerializer, headers, - url: this.#buildUrl(reqOpts.url?.toString(), reqOpts.queryParameters), + url: requestUrl, timeout: this.timeout, + validateStatus: (status: number): boolean => { + const isResumable = !!( + reqOpts.queryParameters?.uploadType === 'resumable' || + reqOpts.url?.toString().includes('uploadType=resumable') + ); + return ( + (status >= 200 && status < 300) || (isResumable && status === 308) + ); + }, }); - return callback - ? requestPromise - .then(resp => callback(null, resp.data, resp)) - .catch(err => callback(err, null, err.response)) - : (requestPromise.then(resp => resp.data) as Promise); + // Response Handling + const responseHandler = (resp: GaxiosResponse) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const data = resp.data as any; + if (data !== null && typeof data === 'object') { + data.headers = resp.headers; + data.status = resp.status; + return data; + } + return resp; + }; + + if (callback) { + requestPromise + .then(resp => callback(null, responseHandler(resp), resp)) + .catch(err => callback(err, null, err.response)); + return; + } + + return requestPromise.then(responseHandler); } catch (e) { if (callback) return callback(e as GaxiosError); throw e; } } - #buildUrl(pathUri = '', queryParameters: StorageQueryParameters = {}): URL { - if ( - 'project' in queryParameters && - (queryParameters.project !== this.projectId || - queryParameters.project !== projectId) - ) { - queryParameters.project = this.projectId; - } - const qp = this.#buildRequestQueryParams(queryParameters); - let url: URL; - if (this.#isValidUrl(pathUri)) { - url = new URL(pathUri); - } else { - url = new URL(`${this.baseUrl}${pathUri}`); + #prepareHeaders(reqOpts: StorageRequestOptions): Record { + const headersObj = this.#buildRequestHeaders(reqOpts.headers); + + if (reqOpts[GCCL_GCS_CMD_KEY]) { + const current = headersObj.get('x-goog-api-client') || ''; + headersObj.set( + 'x-goog-api-client', + `${current} gccl-gcs-cmd/${reqOpts[GCCL_GCS_CMD_KEY]}`, + ); } - url.search = qp; - return url; + const finalHeaders: Record = {}; + headersObj.forEach((v, k) => { + finalHeaders[k] = v; + }); + return finalHeaders; } #isValidUrl(url: string): boolean { @@ -204,32 +232,38 @@ export class StorageTransport { } } + /** + * Serializes query parameters into a string. + * Specifically handles arrays by appending each value individually + * to satisfy GCS "repeated key" requirements (e.g., for IAM permissions). + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + #paramsSerializer = (params: Record): string => { + const searchParams = new URLSearchParams(); + for (const [key, value] of Object.entries(params)) { + if (value === undefined) continue; + + if (Array.isArray(value)) { + value.forEach(v => searchParams.append(key, String(v))); + } else { + searchParams.set(key, String(value)); + } + } + return searchParams.toString(); + }; + #buildRequestHeaders(requestHeaders = {}) { const headers = new Headers(requestHeaders); - headers.set('User-Agent', this.#getUserAgentString()); headers.set( 'x-goog-api-client', `${getRuntimeTrackingString()} gccl/${this.packageJson.version}-${getModuleFormat()} gccl-invocation-id/${randomUUID()}`, ); - return headers; } - #buildRequestQueryParams(queryParameters: StorageQueryParameters): string { - const qp = new URLSearchParams( - queryParameters as unknown as Record, - ); - - return qp.toString(); - } - #getUserAgentString(): string { - let userAgent = getUserAgentString(); - if (this.providedUserAgent) { - userAgent = `${this.providedUserAgent} ${userAgent}`; - } - - return userAgent; + const base = getUserAgentString(); + return this.providedUserAgent ? `${this.providedUserAgent} ${base}` : base; } } diff --git a/handwritten/storage/src/storage.ts b/handwritten/storage/src/storage.ts index d6272cca4018..d825c3b4a6d3 100644 --- a/handwritten/storage/src/storage.ts +++ b/handwritten/storage/src/storage.ts @@ -317,38 +317,111 @@ const IDEMPOTENCY_STRATEGY_DEFAULT = IdempotencyStrategy.RetryConditional; * @return {boolean} True if the API request should be retried, false otherwise. */ export const RETRYABLE_ERR_FN_DEFAULT = function (err?: GaxiosError) { - const isConnectionProblem = (reason: string) => { - return ( - reason.includes('eai_again') || // DNS lookup error - reason === 'econnreset' || - reason === 'unexpected connection closure' || - reason === 'epipe' || - reason === 'socket connection timeout' - ); - }; - - if (err) { - if ([408, 429, 500, 502, 503, 504].indexOf(err.status!) !== -1) { - return true; + if (!err || !err.config) return false; + + const config = err.config; + const method = (config.method || 'GET').toUpperCase(); + const url = config.url ? config.url.toString() : ''; + const params = config.params || {}; + const data = config.data; + + // Immediate exit for non-retryable status codes + const status = err.response?.status; + if (status && [401, 405, 412].includes(status)) return false; + + // Optimized Precondition Check + let bodyEtag = false; + try { + const parsedBody = typeof data === 'string' ? JSON.parse(data) : data; + if (parsedBody && parsedBody.etag) { + bodyEtag = true; } + } catch (e) { + // If parsing fails, we treat it as no etag and move on + bodyEtag = false; + } - if (typeof err.code === 'string') { - if (['408', '429', '500', '502', '503', '504'].indexOf(err.code) !== -1) { - return true; - } - const reason = (err.code as string).toLowerCase(); - if (isConnectionProblem(reason)) { - return true; - } + const hasPrecondition = !!( + params.ifGenerationMatch !== undefined || + params.ifMetagenerationMatch !== undefined || + params.ifSourceGenerationMatch !== undefined || + bodyEtag + ); + + // Granular Idempotency Logic + let isIdempotent = false; + if (['GET', 'HEAD'].includes(method) || hasPrecondition) { + isIdempotent = true; + } else if (method === 'PUT') { + // Resumable uploads (upload_id) are idempotent. + // IAM/HMAC are only idempotent if they have an etag (handled in hasPrecondition). + const isResumable = url.includes('upload_id='); + const isSpecialMutation = + /\/iam($|\?)/.test(url) || /\/hmacKeys\//.test(url); + isIdempotent = isResumable || !isSpecialMutation; + } else if (method === 'DELETE') { + // Deleting a specific object is only idempotent with a precondition. + // Deleting a bucket/HMAC is generally safe to retry. + if (!url.includes('/o/')) { + isIdempotent = true; } + } else if (method === 'POST') { + // Bucket creation is safe to retry. + // Object mutations (rewrite/copy) must have a precondition (handled above). + isIdempotent = + url.includes('/v1/b') && + !url.includes('/o') && + !url.includes('/notificationConfigs'); + } + if (!isIdempotent) return false; + + const gcsErrors = err.response?.data?.error?.errors || []; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const hasRateLimitReason = gcsErrors.some((e: any) => + ['rateLimitExceeded', 'userRateLimitExceeded'].includes(e.reason), + ); + + if (hasRateLimitReason) return true; + + // Unified Error Detection + const retryableCodes = [408, 429, 500, 502, 503, 504]; + const errCode = err.code?.toString().toUpperCase() || ''; + const message = err.message?.toLowerCase() || ''; + + // Check HTTP Status + if (status && retryableCodes.includes(status)) return true; + + // Check Gaxios/Node Error Codes + if (retryableCodes.includes(Number(errCode))) return true; + + const connectionErrors = [ + 'ECONNRESET', + 'EPIPE', + 'ETIMEDOUT', + 'EADDRINUSE', + 'ECONNREFUSED', + 'ENOTFOUND', + 'ENETUNREACH', + 'EAI_AGAIN', + ]; + + if ( + connectionErrors.includes(errCode) || + message.includes('socket hang up') + ) { + return true; + } - if (err) { - const reason = err?.code?.toString().toLowerCase(); - if (reason && isConnectionProblem(reason)) { - return true; - } - } + // Handle malformed responses or stream interruptions + if ( + message.includes('unexpected end of json input') || + message.includes('unexpected token') || + message.includes('operation was aborted') || + message.includes('unexpected connection closure') + ) { + return true; } + return false; }; diff --git a/handwritten/storage/system-test/common.ts b/handwritten/storage/system-test/common.ts deleted file mode 100644 index ae2892dabbcd..000000000000 --- a/handwritten/storage/system-test/common.ts +++ /dev/null @@ -1,134 +0,0 @@ -/*! - * Copyright 2022 Google LLC. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import {before, describe, it} from 'mocha'; -import assert from 'assert'; -import * as http from 'http'; - -import * as common from '../src/nodejs-common/index.js'; - -describe('Common', () => { - // MOCK_HOST_PORT is kept for Service initialization but individual tests - // now use dynamic ports to avoid EADDRINUSE collisions in CI. - const MOCK_HOST_PORT = 8118; - const MOCK_HOST = `http://localhost:${MOCK_HOST_PORT}`; - - describe('Service', () => { - let service: common.Service; - - before(() => { - service = new common.Service({ - baseUrl: MOCK_HOST, - apiEndpoint: MOCK_HOST, - scopes: [], - packageJson: {name: 'tests', version: '1.0.0'}, - }); - }); - - it('should send a request and receive a response', done => { - const mockResponse = 'response'; - const mockServer = new http.Server((req, res) => { - res.end(mockResponse); - }); - - // Listen on port 0 to allow the OS to assign a random available port. - // This prevents "port already in use" errors if tests run in parallel. - mockServer.listen(0, () => { - const port = (mockServer.address() as import('net').AddressInfo).port; - - service.request( - { - uri: `http://localhost:${port}/mock-endpoint`, - }, - (err, resp) => { - try { - assert.ifError(err); - assert.strictEqual(resp, mockResponse); - mockServer.close(done); - } catch (e) { - mockServer.close(() => done(e)); - } - } - ); - }); - }); - - it('should retry a request', function (done) { - // We've increased the timeout to accommodate the retry backoff strategy. - // The test's retry attempts and the delay between them can exceed the default timeout, - // causing a false negative (test failure due to timeout instead of a logic error). - this.timeout(90 * 1000); - - let numRequestAttempts = 0; - - const mockServer = new http.Server((req, res) => { - numRequestAttempts++; - res.statusCode = 408; - res.end(); - }); - - mockServer.listen(0, () => { - const port = (mockServer.address() as import('net').AddressInfo).port; - - service.request( - { - uri: `http://localhost:${port}/mock-endpoint-retry`, - }, - err => { - try { - assert.strictEqual((err! as common.ApiError).code, 408); - assert.strictEqual(numRequestAttempts, 4); - mockServer.close(done); // Ensure done is called only after server is closed - } catch (e) { - mockServer.close(() => done(e)); // Cleanup even if assertion fails - } - } - ); - }); - }); - - it('should retry non-responsive hosts', function (done) { - this.timeout(60 * 1000); - - function getMinimumRetryDelay(retryNumber: number) { - return Math.pow(2, retryNumber) * 1000; - } - - let minExpectedResponseTime = 0; - let numExpectedRetries = 2; - - while (numExpectedRetries--) { - minExpectedResponseTime += getMinimumRetryDelay(numExpectedRetries + 1); - } - - const timeRequest = Date.now(); - - service.request( - { - // Using port :1 (reserved) ensures an immediate ECONNREFUSED - // without risking hitting a real service on the runner. - uri: 'http://localhost:1/mock-endpoint-no-response', - }, - err => { - assert(err?.message.includes('ECONNREFUSED')); - const timeResponse = Date.now(); - assert(timeResponse - timeRequest > minExpectedResponseTime); - done(); - }, - ); - }); - }); -}); diff --git a/handwritten/storage/test/file.ts b/handwritten/storage/test/file.ts index 850f87d4d96e..5a8ec140f398 100644 --- a/handwritten/storage/test/file.ts +++ b/handwritten/storage/test/file.ts @@ -583,19 +583,42 @@ describe('File', () => { it('should set encryption key on the new File instance', done => { // eslint-disable-next-line @typescript-eslint/no-explicit-any - let file: any; - // eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any - file = new (File as any)(BUCKET, FILE_NAME); + const file = new (File as any)(BUCKET, FILE_NAME); + Object.assign(file, { + encryptionKey: 'source-key', + encryptionKeyBase64: 'base64', + encryptionKeyHash: 'hash', + }); // eslint-disable-next-line @typescript-eslint/no-explicit-any const newFile = new (File as any)(BUCKET, 'new-file'); - newFile.encryptionKey = 'encryptionKey'; - - file.setEncryptionKey = sandbox.stub().callsFake(encryptionKey => { - assert.strictEqual(encryptionKey, newFile.encryptionKey); - done(); + Object.assign(newFile, { + encryptionKey: 'dest-key', + encryptionKeyBase64: 'base64-dest', + encryptionKeyHash: 'hash-dest', }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + storageTransport.makeRequest = async (reqOpts: any, callback: any) => { + const actualHeaders = Object.fromEntries(reqOpts.headers.entries()); + + try { + assert.deepStrictEqual(actualHeaders, { + 'content-type': 'application/json', + 'x-goog-copy-source-encryption-algorithm': 'AES256', + 'x-goog-copy-source-encryption-key': 'base64', + 'x-goog-copy-source-encryption-key-sha256': 'hash', + 'x-goog-encryption-algorithm': 'AES256', + 'x-goog-encryption-key': 'base64-dest', + 'x-goog-encryption-key-sha256': 'hash-dest', + }); + callback?.(null, {done: true}, {}); + done(); + } catch (e) { + done(e); + } + }; + file.copy(newFile, assert.ifError); }); @@ -985,6 +1008,7 @@ describe('File', () => { 'Accept-Encoding': 'gzip', 'Cache-Control': 'no-store', }, + decompress: true, responseType: 'stream', queryParameters: { alt: 'media', diff --git a/handwritten/storage/test/index.ts b/handwritten/storage/test/index.ts index 2c9a6a95aa40..ee6d95fbf7c2 100644 --- a/handwritten/storage/test/index.ts +++ b/handwritten/storage/test/index.ts @@ -234,8 +234,15 @@ describe('Storage', () => { const storage = new Storage({ projectId: PROJECT_ID, }); - const error = new GaxiosError('Broken pipe', {} as GaxiosOptionsPrepared); - error.code = 'Socket connection timeout'; + const mockConfig = { + method: 'GET', + url: 'http://127.0.0.1/test', + headers: {}, + } as unknown as GaxiosOptionsPrepared; + + const error = new GaxiosError('socket connection timeout', mockConfig); + + error.code = 'ETIMEDOUT'; assert.strictEqual(storage.retryOptions.retryableErrorFn!(error), true); }); diff --git a/handwritten/storage/test/nodejs-common/service.ts b/handwritten/storage/test/nodejs-common/service.ts deleted file mode 100644 index 502c4e5419f9..000000000000 --- a/handwritten/storage/test/nodejs-common/service.ts +++ /dev/null @@ -1,718 +0,0 @@ -/*! - * Copyright 2022 Google LLC. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import assert from 'assert'; -import {describe, it, before, beforeEach, after} from 'mocha'; -import proxyquire from 'proxyquire'; -import {Request} from 'teeny-request'; -import {AuthClient, GoogleAuth, OAuth2Client} from 'google-auth-library'; - -import {Interceptor} from '../../src/nodejs-common/index.js'; -import { - DEFAULT_PROJECT_ID_TOKEN, - ServiceConfig, - ServiceOptions, -} from '../../src/nodejs-common/service.js'; -import { - BodyResponseCallback, - DecorateRequestOptions, - GCCL_GCS_CMD_KEY, - MakeAuthenticatedRequest, - MakeAuthenticatedRequestFactoryConfig, - util, - Util, -} from '../../src/nodejs-common/util.js'; -import {getUserAgentString, getModuleFormat} from '../../src/util.js'; - -proxyquire.noPreserveCache(); - -const fakeCfg = {} as ServiceConfig; - -const makeAuthRequestFactoryCache = util.makeAuthenticatedRequestFactory; -let makeAuthenticatedRequestFactoryOverride: - | null - | (( - config: MakeAuthenticatedRequestFactoryConfig - ) => MakeAuthenticatedRequest); - -util.makeAuthenticatedRequestFactory = function ( - this: Util, - config: MakeAuthenticatedRequestFactoryConfig -) { - if (makeAuthenticatedRequestFactoryOverride) { - return makeAuthenticatedRequestFactoryOverride.call(this, config); - } - return makeAuthRequestFactoryCache.call(this, config); -}; - -describe('Service', () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let service: any; - const Service = proxyquire('../../src/nodejs-common/service', { - './util': util, - }).Service; - - const CONFIG = { - scopes: [], - baseUrl: 'base-url', - projectIdRequired: false, - apiEndpoint: 'common.endpoint.local', - packageJson: { - name: '@google-cloud/service', - version: '0.2.0', - }, - }; - - const OPTIONS = { - authClient: new GoogleAuth(), - credentials: {}, - keyFile: {}, - email: 'email', - projectId: 'project-id', - token: 'token', - } as ServiceOptions; - - beforeEach(() => { - makeAuthenticatedRequestFactoryOverride = null; - service = new Service(CONFIG, OPTIONS); - }); - - describe('instantiation', () => { - it('should not require options', () => { - assert.doesNotThrow(() => { - new Service(CONFIG); - }); - }); - - it('should create an authenticated request factory', () => { - const authenticatedRequest = {} as MakeAuthenticatedRequest; - - makeAuthenticatedRequestFactoryOverride = ( - config: MakeAuthenticatedRequestFactoryConfig - ) => { - const expectedConfig = { - ...CONFIG, - authClient: OPTIONS.authClient, - credentials: OPTIONS.credentials, - keyFile: OPTIONS.keyFilename, - email: OPTIONS.email, - projectIdRequired: CONFIG.projectIdRequired, - projectId: OPTIONS.projectId, - clientOptions: { - universeDomain: undefined, - }, - }; - - assert.deepStrictEqual(config, expectedConfig); - - return authenticatedRequest; - }; - - const svc = new Service(CONFIG, OPTIONS); - assert.strictEqual(svc.makeAuthenticatedRequest, authenticatedRequest); - }); - - it('should localize the authClient', () => { - const authClient = {}; - makeAuthenticatedRequestFactoryOverride = () => { - return { - authClient, - } as MakeAuthenticatedRequest; - }; - const service = new Service(CONFIG, OPTIONS); - assert.strictEqual(service.authClient, authClient); - }); - - it('should localize the provided authClient', () => { - const service = new Service(CONFIG, OPTIONS); - assert.strictEqual(service.authClient, OPTIONS.authClient); - }); - - describe('`AuthClient` support', () => { - // Using a custom `AuthClient` to ensure any `AuthClient` would work - class CustomAuthClient extends AuthClient { - async getAccessToken() { - return {token: '', res: undefined}; - } - - async getRequestHeaders() { - return {}; - } - - request = OAuth2Client.prototype.request.bind(this); - } - - it('should accept an `AuthClient` passed to config', async () => { - const authClient = new CustomAuthClient(); - const serviceObject = new Service({...CONFIG, authClient}); - - // The custom `AuthClient` should be passed to `GoogleAuth` and used internally - const client = await serviceObject.authClient.getClient(); - - assert.strictEqual(client, authClient); - }); - - it('should accept an `AuthClient` passed to options', async () => { - const authClient = new CustomAuthClient(); - const serviceObject = new Service(CONFIG, {authClient}); - - // The custom `AuthClient` should be passed to `GoogleAuth` and used internally - const client = await serviceObject.authClient.getClient(); - - assert.strictEqual(client, authClient); - }); - }); - - it('should localize the baseUrl', () => { - assert.strictEqual(service.baseUrl, CONFIG.baseUrl); - }); - - it('should localize the apiEndpoint', () => { - assert.strictEqual(service.apiEndpoint, CONFIG.apiEndpoint); - }); - - it('should default the timeout to undefined', () => { - assert.strictEqual(service.timeout, undefined); - }); - - it('should localize the timeout', () => { - const timeout = 10000; - const options = {...OPTIONS, timeout}; - const service = new Service(fakeCfg, options); - assert.strictEqual(service.timeout, timeout); - }); - - it('should default globalInterceptors to an empty array', () => { - assert.deepStrictEqual(service.globalInterceptors, []); - }); - - it('should preserve the original global interceptors', () => { - const globalInterceptors: Interceptor[] = []; - const options = {...OPTIONS}; - options.interceptors_ = globalInterceptors; - const service = new Service(fakeCfg, options); - assert.strictEqual(service.globalInterceptors, globalInterceptors); - }); - - it('should default interceptors to an empty array', () => { - assert.deepStrictEqual(service.interceptors, []); - }); - - it('should localize package.json', () => { - assert.strictEqual(service.packageJson, CONFIG.packageJson); - }); - - it('should localize the projectId', () => { - assert.strictEqual(service.projectId, OPTIONS.projectId); - }); - - it('should default projectId with placeholder', () => { - const service = new Service(fakeCfg, {}); - assert.strictEqual(service.projectId, DEFAULT_PROJECT_ID_TOKEN); - }); - - it('should localize the projectIdRequired', () => { - assert.strictEqual(service.projectIdRequired, CONFIG.projectIdRequired); - }); - - it('should default projectIdRequired to true', () => { - const service = new Service(fakeCfg, OPTIONS); - assert.strictEqual(service.projectIdRequired, true); - }); - - it('should disable forever agent for Cloud Function envs', () => { - process.env.FUNCTION_NAME = 'cloud-function-name'; - const service = new Service(CONFIG, OPTIONS); - delete process.env.FUNCTION_NAME; - - const interceptor = service.interceptors[0]; - - const modifiedReqOpts = interceptor.request({forever: true}); - assert.strictEqual(modifiedReqOpts.forever, false); - }); - }); - - describe('getRequestInterceptors', () => { - it('should call the request interceptors in order', () => { - // Called first. - service.globalInterceptors.push({ - request(reqOpts: {order: string}) { - reqOpts.order = '1'; - return reqOpts; - }, - }); - - // Called third. - service.interceptors.push({ - request(reqOpts: {order: string}) { - reqOpts.order += '3'; - return reqOpts; - }, - }); - - // Called second. - service.globalInterceptors.push({ - request(reqOpts: {order: string}) { - reqOpts.order += '2'; - return reqOpts; - }, - }); - - // Called fourth. - service.interceptors.push({ - request(reqOpts: {order: string}) { - reqOpts.order += '4'; - return reqOpts; - }, - }); - - const reqOpts: {order?: string} = {}; - const requestInterceptors = service.getRequestInterceptors(); - requestInterceptors.forEach((requestInterceptor: Function) => { - Object.assign(reqOpts, requestInterceptor(reqOpts)); - }); - assert.strictEqual(reqOpts.order, '1234'); - }); - - it('should not affect original interceptor arrays', () => { - function request(reqOpts: DecorateRequestOptions) { - return reqOpts; - } - - service.globalInterceptors = [{request}]; - service.interceptors = [{request}]; - - const originalGlobalInterceptors = [].slice.call( - service.globalInterceptors - ); - const originalLocalInterceptors = [].slice.call(service.interceptors); - - service.getRequestInterceptors(); - - assert.deepStrictEqual( - service.globalInterceptors, - originalGlobalInterceptors - ); - assert.deepStrictEqual(service.interceptors, originalLocalInterceptors); - }); - - it('should not call unrelated interceptors', () => { - service.interceptors.push({ - anotherInterceptor() { - throw new Error('Unrelated interceptor was called.'); - }, - request() { - return {}; - }, - }); - - const requestInterceptors = service.getRequestInterceptors(); - requestInterceptors.forEach((requestInterceptor: Function) => { - requestInterceptor(); - }); - }); - }); - - describe('getProjectId', () => { - it('should get the project ID from the auth client', done => { - service.authClient = { - getProjectId() { - done(); - }, - }; - - service.getProjectId(assert.ifError); - }); - - it('should return error from auth client', done => { - const error = new Error('Error.'); - - service.authClient = { - async getProjectId() { - throw error; - }, - }; - - service.getProjectId((err: Error) => { - assert.strictEqual(err, error); - done(); - }); - }); - - it('should update and return the project ID if found', done => { - const service = new Service(fakeCfg, {}); - const projectId = 'detected-project-id'; - - service.authClient = { - async getProjectId() { - return projectId; - }, - }; - - service.getProjectId((err: Error, projectId_: string) => { - assert.ifError(err); - assert.strictEqual(service.projectId, projectId); - assert.strictEqual(projectId_, projectId); - done(); - }); - }); - - it('should return a promise if no callback is provided', () => { - const value = {}; - service.getProjectIdAsync = () => value; - assert.strictEqual(service.getProjectId(), value); - }); - }); - - describe('request_', () => { - let reqOpts: DecorateRequestOptions; - - beforeEach(() => { - reqOpts = { - uri: 'uri', - }; - }); - - it('should compose the correct request', done => { - const expectedUri = [service.baseUrl, reqOpts.uri].join('/'); - service.makeAuthenticatedRequest = ( - reqOpts_: DecorateRequestOptions, - callback: BodyResponseCallback - ) => { - assert.notStrictEqual(reqOpts_, reqOpts); - assert.strictEqual(reqOpts_.uri, expectedUri); - assert.strictEqual(reqOpts.interceptors_, undefined); - callback(null); // done() - }; - service.request_(reqOpts, () => done()); - }); - - it('should support absolute uris', done => { - const expectedUri = 'http://www.google.com'; - - service.makeAuthenticatedRequest = (reqOpts: DecorateRequestOptions) => { - assert.strictEqual(reqOpts.uri, expectedUri); - done(); - }; - - service.request_({uri: expectedUri}, assert.ifError); - }); - - it('should trim slashes', done => { - const reqOpts = { - uri: '//1/2//', - }; - - const expectedUri = [service.baseUrl, '1/2'].join('/'); - - service.makeAuthenticatedRequest = (reqOpts_: DecorateRequestOptions) => { - assert.strictEqual(reqOpts_.uri, expectedUri); - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - - it('should replace path/:subpath with path:subpath', done => { - const reqOpts = { - uri: ':test', - }; - - const expectedUri = service.baseUrl + reqOpts.uri; - service.makeAuthenticatedRequest = (reqOpts_: DecorateRequestOptions) => { - assert.strictEqual(reqOpts_.uri, expectedUri); - done(); - }; - service.request_(reqOpts, assert.ifError); - }); - - it('should not set timeout', done => { - service.makeAuthenticatedRequest = (reqOpts_: DecorateRequestOptions) => { - assert.strictEqual(reqOpts_.timeout, undefined); - done(); - }; - service.request_(reqOpts, assert.ifError); - }); - - it('should set reqOpt.timeout', done => { - const timeout = 10000; - const config = {...CONFIG}; - const options = {...OPTIONS, timeout}; - const service = new Service(config, options); - - service.makeAuthenticatedRequest = (reqOpts_: DecorateRequestOptions) => { - assert.strictEqual(reqOpts_.timeout, timeout); - done(); - }; - service.request_(reqOpts, assert.ifError); - }); - - it('should add the User Agent', done => { - service.makeAuthenticatedRequest = (reqOpts: DecorateRequestOptions) => { - assert.strictEqual( - reqOpts.headers!['User-Agent'], - getUserAgentString() - ); - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - - it('should add the api-client header', done => { - service.makeAuthenticatedRequest = (reqOpts: DecorateRequestOptions) => { - const pkg = service.packageJson; - const r = new RegExp( - `^gl-node/${process.versions.node} gccl/${ - pkg.version - }-${getModuleFormat()} gccl-invocation-id/(?[^W]+)$` - ); - assert.ok(r.test(reqOpts.headers!['x-goog-api-client'])); - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - - it('should add the `gccl-gcs-cmd` to the api-client header when provided', done => { - const expected = 'example.expected/value'; - service.makeAuthenticatedRequest = (reqOpts: DecorateRequestOptions) => { - const pkg = service.packageJson; - const r = new RegExp( - `^gl-node/${process.versions.node} gccl/${ - pkg.version - }-${getModuleFormat()} gccl-invocation-id/(?[^W]+) gccl-gcs-cmd/${expected}$` - ); - assert.ok(r.test(reqOpts.headers!['x-goog-api-client'])); - done(); - }; - - service.request_( - {...reqOpts, [GCCL_GCS_CMD_KEY]: expected}, - assert.ifError - ); - }); - - describe('projectIdRequired', () => { - describe('false', () => { - it('should include the projectId', done => { - const config = {...CONFIG, projectIdRequired: false}; - const service = new Service(config, OPTIONS); - - const expectedUri = [service.baseUrl, reqOpts.uri].join('/'); - - service.makeAuthenticatedRequest = ( - reqOpts_: DecorateRequestOptions - ) => { - assert.strictEqual(reqOpts_.uri, expectedUri); - - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - }); - - describe('true', () => { - it('should not include the projectId', done => { - const config = {...CONFIG, projectIdRequired: true}; - const service = new Service(config, OPTIONS); - - const expectedUri = [ - service.baseUrl, - 'projects', - service.projectId, - reqOpts.uri, - ].join('/'); - - service.makeAuthenticatedRequest = ( - reqOpts_: DecorateRequestOptions - ) => { - assert.strictEqual(reqOpts_.uri, expectedUri); - - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - - it('should use projectId override', done => { - const config = {...CONFIG, projectIdRequired: true}; - const service = new Service(config, OPTIONS); - const projectOverride = 'turing'; - - reqOpts.projectId = projectOverride; - - const expectedUri = [ - service.baseUrl, - 'projects', - projectOverride, - reqOpts.uri, - ].join('/'); - - service.makeAuthenticatedRequest = ( - reqOpts_: DecorateRequestOptions - ) => { - assert.strictEqual(reqOpts_.uri, expectedUri); - - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - }); - }); - - describe('request interceptors', () => { - type FakeRequestOptions = DecorateRequestOptions & {a: string; b: string}; - - it('should include request interceptors', done => { - const requestInterceptors = [ - (reqOpts: FakeRequestOptions) => { - reqOpts.a = 'a'; - return reqOpts; - }, - (reqOpts: FakeRequestOptions) => { - reqOpts.b = 'b'; - return reqOpts; - }, - ]; - - service.getRequestInterceptors = () => { - return requestInterceptors; - }; - - service.makeAuthenticatedRequest = (reqOpts: FakeRequestOptions) => { - assert.strictEqual(reqOpts.a, 'a'); - assert.strictEqual(reqOpts.b, 'b'); - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - - it('should combine reqOpts interceptors', done => { - const requestInterceptors = [ - (reqOpts: FakeRequestOptions) => { - reqOpts.a = 'a'; - return reqOpts; - }, - ]; - - service.getRequestInterceptors = () => { - return requestInterceptors; - }; - - reqOpts.interceptors_ = [ - { - request: (reqOpts: FakeRequestOptions) => { - reqOpts.b = 'b'; - return reqOpts; - }, - }, - ]; - - service.makeAuthenticatedRequest = (reqOpts: FakeRequestOptions) => { - assert.strictEqual(reqOpts.a, 'a'); - assert.strictEqual(reqOpts.b, 'b'); - assert.strictEqual(typeof reqOpts.interceptors_, 'undefined'); - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - }); - - describe('error handling', () => { - it('should re-throw any makeAuthenticatedRequest callback error', done => { - const err = new Error('🥓'); - const res = {body: undefined}; - service.makeAuthenticatedRequest = (_: void, callback: Function) => { - callback(err, res.body, res); - }; - service.request_({uri: ''}, (e: Error) => { - assert.strictEqual(e, err); - done(); - }); - }); - }); - }); - - describe('request', () => { - let request_: Request; - - before(() => { - request_ = Service.prototype.request_; - }); - - after(() => { - Service.prototype.request_ = request_; - }); - - it('should call through to _request', async () => { - const fakeOpts = {}; - Service.prototype.request_ = async (reqOpts: DecorateRequestOptions) => { - assert.strictEqual(reqOpts, fakeOpts); - return Promise.resolve({}); - }; - await service.request(fakeOpts); - }); - - it('should accept a callback', done => { - const fakeOpts = {}; - const response = {body: {abc: '123'}, statusCode: 200}; - Service.prototype.request_ = ( - reqOpts: DecorateRequestOptions, - callback: Function - ) => { - assert.strictEqual(reqOpts, fakeOpts); - callback(null, response.body, response); - }; - - service.request(fakeOpts, (err: Error, body: {}, res: {}) => { - assert.ifError(err); - assert.deepStrictEqual(res, response); - assert.deepStrictEqual(body, response.body); - done(); - }); - }); - }); - - describe('requestStream', () => { - let request_: Request; - - before(() => { - request_ = Service.prototype.request_; - }); - - after(() => { - Service.prototype.request_ = request_; - }); - - it('should return whatever _request returns', async () => { - const fakeOpts = {}; - const fakeStream = {}; - - Service.prototype.request_ = async (reqOpts: DecorateRequestOptions) => { - assert.deepStrictEqual(reqOpts, {shouldReturnStream: true}); - return fakeStream; - }; - - const stream = await service.requestStream(fakeOpts); - assert.strictEqual(stream, fakeStream); - }); - }); -}); diff --git a/handwritten/storage/test/storage-transport.ts b/handwritten/storage/test/storage-transport.ts index 4b71c8fa9d66..f0e0badcad9c 100644 --- a/handwritten/storage/test/storage-transport.ts +++ b/handwritten/storage/test/storage-transport.ts @@ -21,6 +21,7 @@ import {GoogleAuth} from 'google-auth-library'; import sinon from 'sinon'; import assert from 'assert'; import {GCCL_GCS_CMD_KEY} from '../src/nodejs-common/util'; +import {RETRYABLE_ERR_FN_DEFAULT} from '../src/storage'; import {Gaxios} from 'gaxios'; describe('Storage Transport', () => { @@ -46,7 +47,7 @@ describe('Storage Transport', () => { retryDelayMultiplier: 2, maxRetryDelay: 100, totalTimeout: 1000, - retryableErrorFn: () => true, + retryableErrorFn: RETRYABLE_ERR_FN_DEFAULT, }, scopes: ['https://www.googleapis.com/auth/could-platform'], packageJson: {name: 'test-package', version: '1.0.0'}, @@ -58,7 +59,12 @@ describe('Storage Transport', () => { }); it('should make a request with the correct parameters', async () => { - const response = {data: {success: true}}; + const response = { + data: {success: true}, + headers: new Map(), + status: 200, + statusText: 'OK', + }; const requestStub = authClientStub.request as sinon.SinonStub; requestStub.resolves(response); @@ -71,20 +77,19 @@ describe('Storage Transport', () => { assert.strictEqual(requestStub.calledOnce, true); const calledWith = requestStub.getCall(0).args[0]; - assert.strictEqual( - calledWith.url.href, - `${baseUrl}/bucket/object?alt=json&userProject=user-project`, - ); - assert.strictEqual(calledWith.headers.get('content-encoding'), 'gzip'); - assert.ok( - calledWith.headers.get('User-Agent').includes('gcloud-node-storage/'), - ); + assert.strictEqual(calledWith.headers['content-encoding'], 'gzip'); + const headers = calledWith.headers; + const userAgent = headers['User-Agent'] || headers['user-agent']; + assert.ok(userAgent.includes('gcloud-node-storage/')); assert.deepStrictEqual(_response, response.data); }); it('should handle retry options correctly', async () => { const requestStub = authClientStub.request as sinon.SinonStub; - requestStub.resolves({}); + requestStub.resolves({ + data: {}, + headers: new Map(), + }); const reqOpts: StorageRequestOptions = { url: '/bucket/object', }; @@ -105,7 +110,10 @@ describe('Storage Transport', () => { [GCCL_GCS_CMD_KEY]: 'test-key', }; - (authClientStub.request as sinon.SinonStub).resolves({data: {}}); + (authClientStub.request as sinon.SinonStub).resolves({ + data: {}, + headers: new Map(), + }); await transport.makeRequest(reqOpts); @@ -113,14 +121,11 @@ describe('Storage Transport', () => { .args[0]; assert.ok( - calledWith.headers - .get('x-goog-api-client') - .includes('gccl-gcs-cmd/test-key'), + calledWith.headers['x-goog-api-client'].includes('gccl-gcs-cmd/test-key'), ); }); - // TODO: Undo this skip once the gaxios interceptor issue is resolved. - it.skip('should clear and add interceptors if provided', async () => { + it('should clear and add interceptors if provided', async () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const interceptorStub: any = sandbox.stub(); const reqOpts: StorageRequestOptions = { @@ -130,12 +135,31 @@ describe('Storage Transport', () => { const clearStub = sandbox.stub(); const addStub = sandbox.stub(); - (authClientStub.request as sinon.SinonStub).resolves({data: {}}); + const transportInstance = new Gaxios(); transportInstance.interceptors.request.clear = clearStub; transportInstance.interceptors.request.add = addStub; - await transport.makeRequest(reqOpts); + const testTransport = new StorageTransport({ + apiEndpoint: baseUrl, + baseUrl, + authClient: authClientStub, + projectId: 'project-id', + retryOptions: { + maxRetries: 3, + retryDelayMultiplier: 2, + maxRetryDelay: 100, + totalTimeout: 1000, + retryableErrorFn: RETRYABLE_ERR_FN_DEFAULT, + }, + scopes: ['https://www.googleapis.com/auth/could-platform'], + packageJson: {name: 'test-package', version: '1.0.0'}, + gaxiosInstance: transportInstance, + }); + + (authClientStub.request as sinon.SinonStub).resolves({data: {}}); + + await testTransport.makeRequest(reqOpts); assert.strictEqual(clearStub.calledOnce, true); assert.strictEqual(addStub.calledOnce, true); @@ -167,4 +191,207 @@ describe('Storage Transport', () => { const transport = new StorageTransport(options); assert.ok(transport.authClient instanceof GoogleAuth); }); + + it('should handle absolute URLs and project validation', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({url: 'https://my-custom-endpoint.com/v1/b'}); + assert.strictEqual( + requestStub.getCall(0).args[0].url, + 'https://my-custom-endpoint.com/v1/b', + ); + }); + + describe('Storage Transport shouldRetry logic', () => { + it('should retry POST if preconditions are present', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({ + method: 'POST', + url: '/b/bucket/o', + queryParameters: {ifGenerationMatch: 123}, + }); + + const retryConfig = requestStub.getCall(0).args[0].retryConfig; + const error503 = { + response: {status: 503}, + config: { + method: 'POST', + url: '/b/bucket/o', + params: {ifGenerationMatch: 123}, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + + assert.strictEqual(retryConfig.shouldRetry(error503), true); + }); + + it('should retry on malformed JSON responses (SyntaxError)', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({url: '/test'}); + + const retryConfig = requestStub.getCall(0).args[0].retryConfig; + + const malformedError = new Error( + 'Unexpected token < in JSON at position 0', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) as any; + malformedError.stack = 'SyntaxError: Unexpected token <'; + malformedError.config = {method: 'GET', url: '/test'}; + + assert.strictEqual(retryConfig.shouldRetry(malformedError), true); + }); + + it('should retry on 503 for idempotent PUT requests', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({ + method: 'PUT', + url: '/bucket/object', + }); + + const retryConfig = requestStub.getCall(0).args[0].retryConfig; + + const error503 = { + response: {status: 503}, + config: {url: '/bucket/object'}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + + assert.strictEqual(retryConfig.shouldRetry(error503), true); + }); + + it('should NOT retry on 401 Unauthorized', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({url: '/test'}); + + const retryConfig = requestStub.getCall(0).args[0].retryConfig; + + const error401 = { + response: {status: 401}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + + assert.strictEqual(retryConfig.shouldRetry(error401), false); + }); + + it('should treat 308 as a valid status for resumable uploads', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: '308-metadata', headers: new Map()}); + + await transport.makeRequest({ + url: '/upload/storage/v1/b/bucket/o?uploadType=resumable', + queryParameters: {uploadType: 'resumable'}, + }); + + const callArgs = requestStub.getCall(0).args[0]; + + assert.strictEqual(callArgs.validateStatus(308), true); + }); + + it('should retry when GCS reason is rateLimitExceeded', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({url: '/test'}); + const retryConfig = requestStub.getCall(0).args[0].retryConfig; + + const rateLimitError = { + response: { + status: 429, + data: { + error: { + errors: [{reason: 'rateLimitExceeded'}], + }, + }, + }, + config: {method: 'GET', url: '/test'}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + + assert.strictEqual(retryConfig.shouldRetry(rateLimitError), true); + }); + + it('should retry on transient network errors (no response)', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({url: '/test'}); + const retryConfig = requestStub.getCall(0).args[0].retryConfig; + + const connReset = { + code: 'ECONNRESET', + config: {method: 'GET', url: '/test'}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + assert.strictEqual(retryConfig.shouldRetry(connReset), true); + }); + + it('should allow retries for bucket creation and safe deletes', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + await transport.makeRequest({method: 'POST', url: '/v1/b'}); + const retryConfig = requestStub.getCall(0).args[0].retryConfig; + + // No status code (network error) on bucket create should retry + assert.strictEqual( + retryConfig.shouldRetry({ + code: 'ECONNRESET', + config: {method: 'POST', url: '/v1/b'}, + }), + true, + ); + }); + + it('should handle HMAC and IAM retry logic', async () => { + const requestStub = authClientStub.request as sinon.SinonStub; + requestStub.resolves({data: {}, headers: new Map()}); + + // Test HMAC PUT without ETag (should NOT retry) + await transport.makeRequest({ + method: 'PUT', + url: '/hmacKeys/test', + body: JSON.stringify({noEtag: true}), + }); + let retryConfig = requestStub.getCall(0).args[0].retryConfig; + assert.strictEqual( + retryConfig.shouldRetry({ + response: {status: 503}, + config: { + method: 'PUT', + url: '/hmacKeys/test', + data: JSON.stringify({noEtag: true}), + }, + }), + false, + ); + + // Test IAM PUT with ETag (should retry) + await transport.makeRequest({ + method: 'PUT', + url: '/iam/test', + body: JSON.stringify({etag: '123'}), + }); + retryConfig = requestStub.getCall(1).args[0].retryConfig; + assert.strictEqual( + retryConfig.shouldRetry({ + response: {status: 503}, + config: { + method: 'PUT', + url: '/iam/test', + data: JSON.stringify({etag: '123'}), + }, + }), + true, + ); + }); + }); });