fix: don't show beam error to user #203
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We used to show an error to the user when the "error" event of the server sent events source fires. The error message was misleading because it indicated that the error originated from beam, whereas the "error" event only indicates that the server sent events stream was interrupted. When the stream is interrupted the server sent events source tries to reconnect after firing the "error" event. In our case where the server sents events connection is rather short lived (only until we got results from all sites) it makes sense to treat an interruption of the stream as indication that the server closed the connection and not try to reconnect. In fact, there was already a second event handler for the "error" event that did exactly that. So this commit removes the first event handler that shows the error to the user and keeps the second as it was.
torbrenner
approved these changes
Feb 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We used to show an error to the user when the "error" event of the server sent events source fires. The error message was misleading because it indicated that the error originated from beam, whereas the "error" event only indicates that the server sent events stream was interrupted. When the stream is interrupted the server sent events source tries to reconnect after firing the "error" event. In our case where the server sents events connection is rather short lived (only until we got results from all sites) it makes sense to treat an interruption of the stream as indication that the server closed the connection and not try to reconnect. In fact, there was already a second event handler for the "error" event that did exactly that. So this commit removes the first event handler that shows the error to the user and keeps the second as it was.
Fixes #198