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

Handle close message by exiting the transport #6

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

benjaminwood
Copy link
Contributor

@benjaminwood benjaminwood commented Feb 17, 2024

Before this change, the client would end up receiving an empty message when the close message was received. This isn't good because the client expects JSON and an empty string would cause an error.

This change adds a check for the close message and terminates the transport when it is received. This allows the client to retry if needed.

This also has a nice side effect of resolving another error scenario where we weren't properly handling the "unwrap" of the message that has been factored out with this commit.

This change also adds some debug output to help understand the types of messages that are being received. It will not be included in the release build.

🙈 the code definitely needs some help now.

Before this change, the client would end up
receiving an empty message when the close message
was received. This isn't good because the client
expects JSON and an empty string would cause an
error.

This change adds a check for the close message
and terminates the transport when it is received.
This allows the client to retry if needed.

This also has a nice side effect of resolving an
additional error scenario where we weren't properly
handling the "unwrap" of the message that has been
factored out with this commit.

This change also adds some debug output to help
understand the types of messages that are being
received. It will not be included in the release
build.

The code could definitely be cleaned up a bit, but
that's a task for another day once we've gotten a
bit further along.

Co-authored-by: Nate Vick <nate.vick@hint.io>
@benjaminwood benjaminwood self-assigned this Feb 17, 2024
@benjaminwood benjaminwood merged commit 9eb6689 into main Feb 28, 2024
@benjaminwood benjaminwood deleted the handle-close-message branch February 28, 2024 00:58
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.

2 participants