Skip to content
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

websocket: do not error on service's close #672

Merged
merged 1 commit into from
Jan 27, 2021
Merged

websocket: do not error on service's close #672

merged 1 commit into from
Jan 27, 2021

Conversation

tharvik
Copy link
Contributor

@tharvik tharvik commented Jan 4, 2021

When a stream is closed by the service itself (such as when the service was streaming progress and finished with a result), it actually errors with "unexpected error: service finished streaming". As it is quite an expected behavior, I'm removing this error and closing the websocket.

It clashes with #670, as we might want to keep the channel opened, anyway, it doesn't change the current behavior of closing the channel when service finishes, just avoid the error.

@tharvik tharvik self-assigned this Jan 4, 2021
@tharvik tharvik added bug easy review Review that should take less than 30 minutes labels Jan 4, 2021
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 4, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
2.8% 2.8% Duplication

@ineiti
Copy link
Member

ineiti commented Jan 27, 2021

@nkcr can you please merge the PR? I think because it comes from the c4dt repo, it cannot run the travis CI - Branch test, so I cannot merge...

@nkcr nkcr merged commit 3329440 into dedis:master Jan 27, 2021
@tharvik tharvik deleted the streaming_do-not-error-on-close branch February 1, 2021 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug easy review Review that should take less than 30 minutes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants