fix: expose underlying error with timeouts or retries (#1650)

* fix: expose underlying error with timeouts or retries

* Update retries.ts

* run lint

gax/src/normalCalls/retries.ts

@@ -78,11 +78,15 @@ export function retryable(
     // TODO: define A/B testing values for retry behaviors.
 
     /** Repeat the API call as long as necessary. */
-    function repeat() {
+    function repeat(err?: GoogleError) {
       timeoutId = null;
       if (deadline && now.getTime() >= deadline) {
         const error = new GoogleError(
-          `Total timeout of API ${apiName} exceeded ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received.`
+          `Total timeout of API ${apiName} exceeded ${
+            retry.backoffSettings.totalTimeoutMillis
+          } milliseconds ${
+            err ? `retrying error ${err} ` : ''
+          } before any response was received.`
         );
         error.code = Status.DEADLINE_EXCEEDED;
         callback(error);
@@ -91,17 +95,23 @@ export function retryable(
 
       if (retries && retries >= maxRetries) {
         const error = new GoogleError(
-          'Exceeded maximum number of retries before any ' +
-            'response was received'
+          'Exceeded maximum number of retries ' +
+            (err ? `retrying error ${err} ` : '') +
+            'before any response was received'
         );
         error.code = Status.DEADLINE_EXCEEDED;
         callback(error);
         return;
       }
 
       retries++;
+      let lastError = err;
       const toCall = addTimeoutArg(func, timeout!, otherArgs);
       canceller = toCall(argument, (err, response, next, rawResponse) => {
+        // Save only the error before deadline exceeded
+        if (err && err.code !== 4) {
+          lastError = err;
+        }
         if (!err) {
           callback(null, response, next, rawResponse);
           return;
@@ -125,7 +135,7 @@ export function retryable(
             const rpcTimeout = maxTimeout ? maxTimeout : 0;
             const newDeadline = deadline ? deadline - now.getTime() : 0;
             timeout = Math.min(timeoutCal, rpcTimeout, newDeadline);
-            repeat();
+            repeat(lastError);
           }, toSleep);
         }
       });

gax/test/test-application/src/index.ts

@@ -45,6 +45,11 @@ async function testShowcase() {
     gaxServerStreamingRetries: true,
   };
 
+  const grpcClientOptsWithRetries = {
+    grpc,
+    sslCreds: grpc.credentials.createInsecure(),
+  };
+
   const fakeGoogleAuth = {
     getClient: async () => {
       return {
@@ -78,10 +83,16 @@ async function testShowcase() {
   const grpcSequenceClientWithServerStreamingRetries =
     new SequenceServiceClient(grpcClientOptsWithServerStreamingRetries);
 
+  const grpcSequenceClientWithRetries = new SequenceServiceClient(
+    grpcClientOptsWithRetries
+  );
+
   const restClient = new EchoClient(restClientOpts);
   const restClientCompat = new EchoClient(restClientOptsCompat);
 
   // assuming gRPC server is started locally
+  await testEchoErrorWithRetries(grpcSequenceClientWithRetries);
+  await testEchoErrorWithTimeout(grpcSequenceClientWithRetries);
   await testEcho(grpcClient);
   await testEchoError(grpcClient);
   await testExpand(grpcClient);
@@ -201,6 +212,33 @@ function createStreamingSequenceRequestFactory(
   return request;
 }
 
+function createSequenceRequestFactory(
+  statusCodeList: Status[],
+  delayList: number[]
+) {
+  const request = new protos.google.showcase.v1beta1.CreateSequenceRequest();
+  const sequence = new protos.google.showcase.v1beta1.Sequence();
+
+  for (let i = 0; i < statusCodeList.length; i++) {
+    const delay = new protos.google.protobuf.Duration();
+    delay.seconds = delayList[i];
+
+    const status = new protos.google.rpc.Status();
+    status.code = statusCodeList[i];
+    status.message = statusCodeList[i].toString();
+
+    const response = new protos.google.showcase.v1beta1.Sequence.Response();
+    response.delay = delay;
+    response.status = status;
+
+    sequence.responses.push(response);
+  }
+
+  request.sequence = sequence;
+
+  return request;
+}
+
 async function testEcho(client: EchoClient) {
   const request = {
     content: 'test',
@@ -281,6 +319,99 @@ async function testEchoError(client: EchoClient) {
   }
 }
 
+async function testEchoErrorWithRetries(client: SequenceServiceClient) {
+  const backoffSettings = createBackoffSettings(
+    100,
+    1.2,
+    1000,
+    null,
+    1.5,
+    3000,
+    null
+  );
+  const retryOptions = new RetryOptions([14, 4], backoffSettings);
+  backoffSettings.maxRetries = 2;
+
+  const settings = {
+    retry: retryOptions,
+  };
+
+  client.initialize();
+
+  const request = createSequenceRequestFactory(
+    [
+      Status.UNAVAILABLE, // Error code 14
+      Status.UNAVAILABLE,
+      Status.UNAVAILABLE,
+      Status.UNAVAILABLE,
+    ],
+    [0.1, 0.1, 0.1, 0.1]
+  );
+
+  const response = await client.createSequence(request);
+  const sequence = response[0];
+
+  const attemptRequest =
+    new protos.google.showcase.v1beta1.AttemptSequenceRequest();
+  attemptRequest.name = sequence.name!;
+
+  try {
+    await client.attemptSequence(attemptRequest, settings);
+  } catch (err) {
+    assert.strictEqual(JSON.stringify((err as GoogleError).code), '4');
+    assert.match(
+      JSON.stringify((err as GoogleError).message),
+      /Exceeded maximum number of retries retrying error Error: 14 UNAVAILABLE: 14 before any response was received/
+    );
+  }
+}
+
+async function testEchoErrorWithTimeout(client: SequenceServiceClient) {
+  const backoffSettings = createBackoffSettings(
+    100,
+    1.2,
+    1000,
+    null,
+    1.5,
+    3000,
+    1
+  );
+  const retryOptions = new RetryOptions([14, 4], backoffSettings);
+
+  const settings = {
+    retry: retryOptions,
+  };
+
+  client.initialize();
+
+  const request = createSequenceRequestFactory(
+    [
+      Status.UNAVAILABLE, // Error code 14
+      Status.UNAVAILABLE,
+      Status.UNAVAILABLE,
+      Status.UNAVAILABLE,
+    ],
+    [0.1, 0.1, 0.1, 0.1]
+  );
+
+  const response = await client.createSequence(request);
+  const sequence = response[0];
+
+  const attemptRequest =
+    new protos.google.showcase.v1beta1.AttemptSequenceRequest();
+  attemptRequest.name = sequence.name!;
+
+  try {
+    await client.attemptSequence(attemptRequest, settings);
+  } catch (err) {
+    assert.strictEqual(JSON.stringify((err as GoogleError).code), '4');
+    assert.match(
+      JSON.stringify((err as GoogleError).message),
+      /Total timeout of API google.showcase.v1beta1.SequenceService exceeded 1 milliseconds retrying error Error: 14 UNAVAILABLE: 14 {2}before any response was received./
+    );
+  }
+}
+
 async function testExpand(client: EchoClient) {
   const words = ['nobody', 'ever', 'reads', 'test', 'input'];
   const request = {
@@ -847,7 +978,7 @@ async function testShouldFailOnThirdError(client: SequenceServiceClient) {
   });
 }
 
-// streaming call that retries twice with RetryRequestOpsions and resumes from where it left off
+// streaming call that retries twice with RetryRequestOptions and resumes from where it left off
 async function testServerStreamingRetrieswithRetryRequestOptionsResumptionStrategy(
   client: SequenceServiceClient
 ) {

gax/test/unit/apiCallable.ts

@@ -551,8 +551,7 @@ describe('retryable', () => {
     });
   });
 
-  // maxRetries is unsupported, and intended for internal use only.
-  it('errors on maxRetries', done => {
+  it('errors on maxRetries and surfaces original error', done => {
     const toAttempt = 5;
     const backoff = gax.createMaxRetriesBackoffSettings(
       0,
@@ -574,18 +573,22 @@ describe('retryable', () => {
       assert.ok(err instanceof GoogleError);
       assert.strictEqual(err!.code, status.DEADLINE_EXCEEDED);
       assert.strictEqual(spy.callCount, toAttempt);
+      assert.match(
+        err.message,
+        /Exceeded maximum number of retries retrying error Error before any response was received/
+      );
       done();
     });
   });
 
-  it('retry fails for exceeding total timeout', done => {
+  it('retry fails for exceeding total timeout, surfacing original error', done => {
     const spy = sinon.spy(fail);
     const apiCall = createApiCall(spy, settings);
     apiCall({}, undefined, err => {
       assert.ok(err instanceof GoogleError);
-      assert.strictEqual(
+      assert.match(
         err.message,
-        'Total timeout of API TestApi exceeded 100 milliseconds before any response was received.'
+        /Total timeout of API TestApi exceeded 100 milliseconds retrying error Error {2}before any response was received/
       );
       assert.strictEqual(err!.code, status.DEADLINE_EXCEEDED);
       done();