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

Recycle last take from state chan if ws write failed #178

Merged
merged 1 commit into from
Jun 3, 2020

Conversation

eidekrist
Copy link
Member

Fixes #143, no module metadata after browser refresh.

During a browser refresh event the websocket connection is closed, then created anew. On the server there is a loop which hangs on the state chan. During the refresh, the loop takes a value from the state chan, and exits when it fails to push the message on the now closed websocket connection. This means that on every refresh we lost the very first command response containing the model metadata.

The solution is pretty simple; if the websocket write fails, put latestState back on the state chan.

@eidekrist eidekrist added the bug Something isn't working label Jun 2, 2020
@eidekrist eidekrist self-assigned this Jun 2, 2020
@eidekrist eidekrist changed the title #143 Recycle last take from state chan if write failed. Recycle last take from state chan if ws write failed Jun 2, 2020
Copy link
Member

@ljamt ljamt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix! 😄
Confirming that it solves scenario 1 of #143

Copy link
Contributor

@hplatou hplatou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@eidekrist eidekrist merged commit e9ad6df into master Jun 3, 2020
@eidekrist eidekrist deleted the bugfix/143-recycle-state-after-ws-close branch June 3, 2020 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues after browser refresh
3 participants