fix(storage): unit test

Author: thiyaguk09

handwritten/storage/conformance-test/conformanceCommon.ts

@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-// eslint-disable-next-line @typescript-eslint/no-unused-vars
 import * as jsonToNodeApiMapping from './test-data/retryInvocationMap.json';
 import * as libraryMethods from './libraryMethods';
 import {Bucket, File, Gaxios, HmacKey, Notification, Storage} from '../src';
@@ -138,6 +137,19 @@ export function executeScenario(testCase: RetryTestCase) {
               throw new Error('Failed to get a valid test ID from test bench.');
             }
 
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            const internalGaxios = (storageTransport as any).authClient
+              ?.gaxiosInstance;
+
+            if (internalGaxios) {
+              // eslint-disable-next-line @typescript-eslint/no-explicit-any
+              internalGaxios.interceptors.request.use((config: any) => {
+                config.headers = config.headers || {};
+                config.headers['x-retry-test-id'] = creationResult.id;
+                return config;
+              });
+            }
+
             // Create a Proxy around rawStorageTransport to intercept makeRequest
             storageTransport = new Proxy(rawStorageTransport, {
               get(target, prop, receiver) {

handwritten/storage/conformance-test/libraryMethods.ts

@@ -121,8 +121,15 @@ export async function create(options: ConformanceTestOptions) {
     method: 'GET',
     url: `storage/v1/b/${bucketName}`,
   };
-  await options.storageTransport.makeRequest(existsReq);
-  bucketExists = true;
+  try {
+    await options.storageTransport.makeRequest(existsReq);
+    bucketExists = true;
+  } catch (e) {
+    const err = e as GaxiosError;
+    if (err.response?.status !== 404) {
+      throw e;
+    }
+  }
 
   if (bucketExists) {
     let pageToken: string | undefined = undefined;

handwritten/storage/src/resumable-upload.ts

@@ -1208,6 +1208,8 @@ export class Upload extends Writable {
         code: resp.status.toString(),
         message: resp.statusText,
         name: resp.statusText,
+        config: resp.config,
+        response: resp,
       } as GaxiosError)
     ) {
       void this.attemptDelayedRetry(resp);

handwritten/storage/src/storage-transport.ts

@@ -174,7 +174,9 @@ export class StorageTransport {
           noResponseRetries: this.retryOptions.maxRetries,
           maxRetryDelay: this.retryOptions.maxRetryDelay,
           retryDelayMultiplier: this.retryOptions.retryDelayMultiplier,
-          shouldRetry: this.retryOptions.retryableErrorFn,
+          shouldRetry: err => {
+            return !!this.retryOptions.retryableErrorFn?.(err);
+          },
           totalTimeout: this.retryOptions.totalTimeout,
         },
         ...reqOpts,

handwritten/storage/src/storage.ts

@@ -306,25 +306,55 @@ export const RETRYABLE_ERR_FN_DEFAULT = function (err?: GaxiosError) {
   if (!err || !err.config) return false;
 
   const method = (err.config.method || 'GET').toUpperCase();
+  const url = err.config.url || '';
   const params = err.config.params || {};
 
+  const status = err.response?.status;
+  if (status && [401, 405, 412].includes(status)) return false;
+
   const hasPrecondition = !!(
     params.ifGenerationMatch !== undefined ||
     params.ifMetagenerationMatch !== undefined ||
     params.ifSourceGenerationMatch !== undefined
   );
 
-  const isIdempotent =
-    ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(method) || !!hasPrecondition;
+  const isGet = method === 'GET';
+  const isHead = method === 'HEAD';
+  const isPut = method === 'PUT';
+  const isDelete = method === 'DELETE';
+  const isPost = method === 'POST';
+
+  let isIdempotent = false;
+  if (isGet || isHead) {
+    isIdempotent = true;
+  } else if (hasPrecondition) {
+    isIdempotent = true;
+  } else if (isDelete) {
+    if (!url.toString().includes('/o/')) {
+      isIdempotent = true;
+    }
+  } else if (isPut) {
+    const isSpecialMutation =
+      url.toString().includes('/iam') || url.toString().includes('/hmacKeys/');
+    if (!isSpecialMutation) {
+      isIdempotent = true;
+    }
+  } else if (isPost) {
+    if (url.toString().includes('/o') || url.toString().includes('/v1/b')) {
+      isIdempotent = true;
+    }
+  }
   if (!isIdempotent) return false;
 
   const isConnectionProblem = (reason: string) => {
     return (
-      reason.includes('eai_again') || // DNS lookup error
+      reason.includes('eai_again') ||
       reason === 'econnreset' ||
       reason === 'unexpected connection closure' ||
       reason === 'epipe' ||
-      reason === 'socket connection timeout'
+      reason === 'socket connection timeout' ||
+      reason === 'econnrefused' ||
+      reason === 'etimedout'
     );
   };
 
@@ -349,6 +379,14 @@ export const RETRYABLE_ERR_FN_DEFAULT = function (err?: GaxiosError) {
         return true;
       }
     }
+
+    const message = (err.message || '').toLowerCase();
+    if (
+      message.includes('unexpected end of json input') ||
+      message.includes('the operation was aborted')
+    ) {
+      return true;
+    }
   }
   return false;
 };

handwritten/storage/test/resumable-upload.ts

@@ -1931,7 +1931,15 @@ describe('resumable-upload', () => {
     });
 
     describe('500s', () => {
-      const RESP = {status: 500, data: 'error message from server'};
+      const RESP = {
+        status: 500,
+        statusText: 'Internal Server Error',
+        data: 'error message from server',
+        config: {
+          method: 'POST',
+          url: `${BASE_URI}/${BUCKET}/o`,
+        },
+      };
 
       it('should increase the retry count if less than limit', () => {
         up.getRetryDelay = () => 1;