gax-nodejs icon indicating copy to clipboard operation
gax-nodejs copied to clipboard

fix: don't emit error event during stream handoff

Open leahecole opened this issue 1 year ago • 1 comments

fixes b/338064353

OLD BEHAVIOR: in streamingRetryRequest.ts as part of onResponse, an 'error' event would be emitted while the retryStream is being destroyed. This event (usually the first error event in a retry sequence) would be bubbled up to the caller as part of the stream handoff that happens in retries where the old stream is destroyed, a new one is created, and events are forwarded. The call would still resume afterwards and continue successfully, but that error would be raised when it shouldn't have been

NEW BEHAVIOR: The stream is still destroyed, but no error event is emitted. Stream handoff is the same as before, call continues, and the only errors that would be bubbled up to the caller should be ones that are not retry eligible.

This PR makes that one line behavior change and adjusts the test-application to expect that behavior. Our tests beforehand were doing error handling that is atypical of what a client would do (we were either ignoring errors or checking info about them before raising them, which should be handled at the gax level)

Major thanks to @danieljbruce for noticing the resulting behavior and raising it.

leahecole avatar May 02 '24 17:05 leahecole

I'll take a look at the system test failure on Monday!

leahecole avatar May 03 '24 17:05 leahecole