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

don't crash ws connection when there's an internal error #245

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

tpolecat
Copy link
Member

The previous behavior here is that the websocket handler would crash and disconnect if a GraphQLService result indicated an internal error, but this isn't really what we want. This changes the handling to return a WebSocket error message in that case instead.

@mergify mergify bot added the core label Apr 24, 2024
@tpolecat tpolecat requested a review from swalker2m April 24, 2024 18:21
Copy link
Contributor

@swalker2m swalker2m left a comment

Choose a reason for hiding this comment

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

This seems right to me. Thanks for handling it. 👍

@swalker2m
Copy link
Contributor

I wonder, for an internal error, if we should also be logging it?

@tpolecat
Copy link
Member Author

tpolecat commented Apr 24, 2024

I wonder, for an internal error, if we should also be logging it?

I'm will log it at a lower level in the ODB, so I think we'll be ok.

@tpolecat tpolecat merged commit 24c4210 into main Apr 24, 2024
6 checks passed
@tpolecat tpolecat deleted the error-handling branch April 24, 2024 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants