From f4d037ac6638b5e9eeb2ee608fe1d8e4cb9a19d3 Mon Sep 17 00:00:00 2001 From: sofisl <55454395+sofisl@users.noreply.github.com> Date: Wed, 4 Sep 2024 08:20:19 -0700 Subject: [PATCH] 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 | 20 +++- gax/test/test-application/src/index.ts | 133 ++++++++++++++++++++++++- gax/test/unit/apiCallable.ts | 13 ++- 3 files changed, 155 insertions(+), 11 deletions(-) diff --git a/gax/src/normalCalls/retries.ts b/gax/src/normalCalls/retries.ts index 2377b1360..ed67a4679 100644 --- a/gax/src/normalCalls/retries.ts +++ b/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,8 +95,9 @@ 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); @@ -100,8 +105,13 @@ export function retryable( } 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); } }); diff --git a/gax/test/test-application/src/index.ts b/gax/test/test-application/src/index.ts index 51bdd7a47..5a036f74f 100644 --- a/gax/test/test-application/src/index.ts +++ b/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 ) { diff --git a/gax/test/unit/apiCallable.ts b/gax/test/unit/apiCallable.ts index ae98bbf41..4a17d590b 100644 --- a/gax/test/unit/apiCallable.ts +++ b/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();