-
Notifications
You must be signed in to change notification settings - Fork 147
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: retry Query streams #1116
fix: retry Query streams #1116
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dev/src/reference.ts
Outdated
|
||
let active = true; | ||
while (active) { | ||
const deferred = new Deferred<boolean>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What condition does deferred
represent? Naming this something that ties it to its meaning more than its type would be helpful. Based on my read it seems like maybe shouldRetry
, retryRequired
, or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merged this with the active
flag and renamed to streamActive
.
…-firestore into mrschmidt/retrystreams
This is one of the ideas I discussed with clem@ on how to make Query streams more robust. Basically, we can restart a stream with the last received document as the cursor.