Skip to content

Commit

Permalink
fix: review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
olavloite committed Jan 13, 2020
1 parent 67da57e commit 569570f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
8 changes: 5 additions & 3 deletions src/partial-result-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ export function partialResultStream(
): PartialResultStream {
const retryableCodes = [status.UNAVAILABLE];
let lastResumeToken: ResumeToken;
let lastErr: ServiceError | undefined;
let lastRetriedErr: ServiceError | undefined;
let lastRequestStream: Readable;

// mergeStream allows multiple streams to be connected into one. This is good;
Expand All @@ -362,7 +362,7 @@ export function partialResultStream(
// This listener ensures that the last request that executed successfully
// after one or more retries will end the requestsStream.
const endListener = () => {
if (lastErr) {
if (lastRetriedErr) {
setImmediate(() => requestsStream.end());
}
};
Expand All @@ -385,6 +385,9 @@ export function partialResultStream(
}

// We're going to retry from where we left off.
// Keep track of the fact that we retried an error in order to end the
// merged result stream.
lastRetriedErr = err;
// Empty queued rows on the checkpoint stream (will not emit them to user).
batchAndSplitOnTokenStream.reset();
makeRequest();
Expand All @@ -396,7 +399,6 @@ export function partialResultStream(
// need types for events-intercept
// tslint:disable-next-line no-any
(requestsStream as any).intercept('error', err => {
lastErr = err;
retry(err);
});

Expand Down
5 changes: 4 additions & 1 deletion test/partial-result-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ describe('PartialResultStream', () => {
});

requestFnStub.onCall(1).callsFake(resumeToken => {
assert.ok(!resumeToken, 'Retry called with resume token');
assert.ok(
!resumeToken,
'Retry should be called with empty resume token'
);

setTimeout(() => {
secondFakeRequestStream.push(RESULT_WITH_TOKEN);
Expand Down

0 comments on commit 569570f

Please sign in to comment.