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

fix an issue where websocket crashes on internal control closeframe #25

Merged
merged 2 commits into from
Sep 18, 2023
Merged

fix an issue where websocket crashes on internal control closeframe #25

merged 2 commits into from
Sep 18, 2023

Conversation

eliknebel
Copy link
Contributor

@eliknebel eliknebel commented Sep 17, 2023

Potentially fixes an issue where websocket crashes on internal control closeframe.

The assertion being made in src/mist/internal/websocket.gleam:232 crashes a webserver using mist websockets when the call returns an error.

This fix appears to conform to the patterns in other case clauses, where there is no assertion made to the result from connection.transport.send. Given a better understanding of the purpose of this function however, there may be a better way to approach this.

Closes #24

@rawhat
Copy link
Owner

rawhat commented Sep 18, 2023

I think to your point about the other methods... we should probably attempt to bubble that up to the caller. For this one, however, if the socket receives a close frame, I'm fairly certain the client is going to close regardless.

I'll need to (re-)check the spec, but I believe it's not expected to necessarily expect it to complete. Either way, this one looks correct to me.

Thank you!

@rawhat
Copy link
Owner

rawhat commented Sep 18, 2023

Do you mind adding an entry to the CHANGELOG please? Then I can get this merged in :)

@eliknebel
Copy link
Contributor Author

Do you mind adding an entry to the CHANGELOG please? Then I can get this merged in :)

Sure thing!

@rawhat rawhat merged commit 65baa70 into rawhat:master Sep 18, 2023
1 check failed
@rawhat
Copy link
Owner

rawhat commented Sep 18, 2023

Awesome, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ok(_) assertion in mist/internal/websocket.gleam crashes process
2 participants