Skip to content

Commit

Permalink
fix: Only call getResumptionRequestFn once (#1593)
Browse files Browse the repository at this point in the history
* Only call getResumptionRequestFn once

The current code calls getResumptionRequestFn twice when the result from the first call can be reused. For clients implementing retries, we only want one call to getResumptionRequestFn to be done per request because we don’t want the state that getResumptionRequestFn relies on to be modified twice.

* fix: Only call getResumptionRequestFn once

* Add a sinon spy to make sure the fn is called once

* Remove only

* ran linter

---------

Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com>
  • Loading branch information
danieljbruce and leahecole committed May 9, 2024
1 parent 2e7d30a commit e1755a9
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
2 changes: 1 addition & 1 deletion gax/src/streamingCalls/streaming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export class StreamProxy extends duplexify implements GRPCCallResult {
const resumptionRetryArgument =
retry.getResumptionRequestFn(retryArgument);
if (resumptionRetryArgument !== undefined) {
retryArgument = retry.getResumptionRequestFn(retryArgument);
retryArgument = resumptionRetryArgument;
}
}
this.resetStreams(stream);
Expand Down
5 changes: 3 additions & 2 deletions gax/test/unit/streaming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1157,10 +1157,10 @@ describe('streaming', () => {
true // new retry behavior enabled
);
// resumption strategy is to pass a different arg to the function
const getResumptionRequestFn = (originalRequest: RequestType) => {
const getResumptionRequestFn = sinon.spy((originalRequest: RequestType) => {
assert.strictEqual(originalRequest.arg, 0);
return {arg: 2};
};
});
const s = apiCall(
{arg: 0},
{
Expand Down Expand Up @@ -1194,6 +1194,7 @@ describe('streaming', () => {
receivedData.join(' '),
'Hello World testing retries'
);
assert.strictEqual(getResumptionRequestFn.callCount, 1);
done();
});
});
Expand Down

0 comments on commit e1755a9

Please sign in to comment.