-
Notifications
You must be signed in to change notification settings - Fork 310
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 event stream error handling and reconnection #6749
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.
Based on my local test, this seems to work.
Still working on some fine-tuning here. Will merge later today. |
89b8d75
to
26c49f7
Compare
26c49f7
to
907f52f
Compare
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've not got that much context for this so i've mostly spent some time to look over how this all works. Nothing to add as I think I need more time to really get my head into this. I can put more time daily to catch up with all this.
Summary
This PR fixes error handling and reconnection logic of event streams.
Changes
Testing
Local testing.
Notes for Reviewers
This is a somewhat preliminary fix to get the reconnects working again. Error handling in general needs some more love since error propagation is now quite different under the WebSocket logic than it has been before. Concretely we should look into how to identify special errors such as authentication and network errors when using the WebSocket connection to apply special logic in these cases.
Checklist
README.md
for the chosen target branch.CHANGELOG.md
.CONTRIBUTING.md
, there are no fixup commits left.