Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: raise error on retryable error when max retries is 0 #1605

Merged
merged 4 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions gax/src/streamingCalls/streaming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,12 @@ export class StreamProxy extends duplexify implements GRPCCallResult {
return; // end chunk
}
} else {
if (maxRetries === 0) {
const e = GoogleError.parseGRPCStatusDetails(error);
e.note = 'Max retries is set to zero.';
this.destroy(e);
return; // end chunk
}
return GoogleError.parseGRPCStatusDetails(error);
}
});
Expand Down
81 changes: 78 additions & 3 deletions gax/test/test-application/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
GoogleAuth,
Status,
createBackoffSettings,
createMaxRetriesBackoffSettings,
RetryOptions,
} from 'google-gax';
import {RequestType} from 'google-gax/build/src/apitypes';
Expand Down Expand Up @@ -108,6 +109,7 @@
await testChatThrows(restClientCompat); // REGAPIC does not support bidi streaming
await testWait(restClientCompat);
// Testing with gaxServerStreamingRetries being true

await testServerStreamingRetryOptions(
grpcSequenceClientWithServerStreamingRetries
);
Expand Down Expand Up @@ -144,6 +146,12 @@
grpcSequenceClientWithServerStreamingRetries
);

await testShouldFailOnThirdError(
grpcSequenceClientWithServerStreamingRetries
);

await testErrorMaxRetries0(grpcSequenceClientWithServerStreamingRetries);
// ensure legacy tests pass with streaming retries client
await testEcho(grpcClientWithServerStreamingRetries);
await testEchoError(grpcClientWithServerStreamingRetries);
await testExpand(grpcClientWithServerStreamingRetries);
Expand All @@ -152,9 +160,6 @@
await testCollect(grpcClientWithServerStreamingRetries);
await testChat(grpcClientWithServerStreamingRetries);
await testWait(grpcClientWithServerStreamingRetries);
await testShouldFailOnThirdError(
grpcSequenceClientWithServerStreamingRetries
);
}

function createStreamingSequenceRequestFactory(
Expand Down Expand Up @@ -1146,6 +1151,76 @@
});
}

// The test should not retry when the max retries are set to 0
// and the emitted error should bubble up to the user when it does not retry.
async function testErrorMaxRetries0(client: SequenceServiceClient) {
const finalData: string[] = [];

Check warning on line 1157 in gax/test/test-application/src/index.ts

View workflow job for this annotation

GitHub Actions / lint-gax

'finalData' is assigned a value but never used
const shouldRetryFn = (error: GoogleError) => {
return [4].includes(error!.code!);
};
const backoffSettings = createMaxRetriesBackoffSettings(
10000,
2.5,
1000,
0,
1.5,
3000,
0
);
const getResumptionRequestFn = (request: RequestType) => {
return request;
};

const retryOptions = new RetryOptions(
[],
backoffSettings,
shouldRetryFn,
getResumptionRequestFn
);

const settings = {
retry: retryOptions,
};

client.initialize();

const request = createStreamingSequenceRequestFactory(
[Status.DEADLINE_EXCEEDED, Status.OK],
[0.1, 0.1],
[0, 1],
'This is testing the brand new and shiny StreamingSequence server 3'
);
const response = await client.createStreamingSequence(request);
await new Promise<void>((resolve, reject) => {
const sequence = response[0];

const attemptRequest =
new protos.google.showcase.v1beta1.AttemptStreamingSequenceRequest();
attemptRequest.name = sequence.name!;
const attemptStream = client.attemptStreamingSequence(
attemptRequest,
settings
);
attemptStream.on('data', () => {
reject(new GoogleError('The stream should not receive any data'));
});
attemptStream.on('error', (error: GoogleError) => {
try {
assert.strictEqual(error.code, 4);
assert.strictEqual(error.note, 'Max retries is set to zero.');
resolve();
} catch (assertionError: unknown) {
reject(assertionError);
}
});
attemptStream.on('end', () => {
reject(
new GoogleError('The stream should not end before it receives an error')
);
});
});
}

async function main() {
const showcaseServer = new ShowcaseServer();
try {
Expand Down
Loading