Skip to content

Commit

Permalink
fix: overhaul of server streaming retries (#1653)
Browse files Browse the repository at this point in the history
* fix: reduce duplicate code in streaming retries and add a test

* add comments

* WIP experiment

* experiments

* WIP - loss example

* fix typo, use pipe not pumpify

* example of piping errors together

* pipeline example

* experiment continued

* no buffer, data loss

* stream pipeline example

* example that shows before and after

* set immediate exampel

* call both tests

* another scenario

* more experiment

* more experimentation

* more experimenting

* some reorganization

* more experiments

* WIP: more exampels

* WIP

* WIP: fixing tests

* move streamingRetryRequest to streaming.ts, remove delayStream

* WIP - move SRR into streamProxy class

* WIP - reduce duplicate code between forwardEventsREtries and streamingErrorhanodffhandler

* WIP - exploration of SRR removal

* merge with cbt2

* fix mistake from rebase

* wip - remove bind from non error  emit

* WIP - stream retries pipeline - success case

* WIP - redoing SRR, successful calls

* wip - redoing retries, fails immediately

* WIP - some retrying

* WIP - theoretically working

* some test cleanup

* WIP

* WIP

* WIP - resetting retries to zero

* change .end to .destroy

* Working version of new retries

* linted test app

* WIP - no lint errors, but lots commented out

* cleanup

* remove files that shouldn't be here

* remove files

* remove protos that don't need to be there

* remove more files

* remove text files

* more cleanup

* remove more files

* remove old directories

* finish file cleanup

* more cleanup

* fix one unit test

* fix some unit tests

* revert previous change

* fix some unit tests

* fix another unit test

* fix one more unit test

* WIP - event forwarding is working!

* fix test behavior - move data push inside of case

* WIP - fixing shouldRetry test

* fix another unit test

* fix another unit test

* fix two more tests

* fix more unit tests, remove console logs

* WIP - trying final test again

* add a lil stream destruction

* WIP - update returns in inner function

* remove return

* add status/end boolean, remove retrying boolean

* some testing fixes

* more fixes

* lots of lint fixes

* fix todos, fix test behavior

* some lint fixes, some todo cleanups

* resolve some todos

* add a test, todo cleanup

* WIP - timeout stuff

* Timeout test

* lint fixes

* remove retries param

* helper function

* resolving todos

* resolve lint

* clean up tests

* final fix of tests

* re-add accidentally deleted file

* remove erroneous print statement

* private function

* address review comment

* fixing docstring

* fix lint

* add comments in test

* type coercion change

* resolve review comment

* better typing in unit tests

* fix lint

* fix - make sure package.json matches main

* try/catch for timeout test

* fix lint
  • Loading branch information
leahecole authored Sep 24, 2024
1 parent 564e9e7 commit 629f4c4
Show file tree
Hide file tree
Showing 7 changed files with 2,278 additions and 792 deletions.
9 changes: 9 additions & 0 deletions gax/src/gax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,15 @@ export function convertRetryOptions(
// this is in seconds and needs to be converted to milliseconds and the totalTimeoutMillis parameter
if (options.retryRequestOptions.totalTimeout !== undefined) {
totalTimeoutMillis = options.retryRequestOptions.totalTimeout * 1000;
} else {
if (options.maxRetries === undefined) {
totalTimeoutMillis = 30000;
warn(
'retry_request_options_no_max_retries_timeout',
'Neither maxRetries nor totalTimeout were passed. Defaulting to totalTimeout of 30000ms.',
'MissingParameterWarning'
);
}
}

// for the variables the user wants to override, override in the backoff settings object we made
Expand Down
Loading

0 comments on commit 629f4c4

Please sign in to comment.