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 (graphql-middleware): Client can't send new graphql queries after a while #18895

Conversation

gustavotrott
Copy link
Collaborator

No description provided.

@gustavotrott gustavotrott added this to the Release 3.0 milestone Oct 4, 2023
@gustavotrott gustavotrott changed the title fix: Client can't send new graphql queries after a while fix (graphql-middleware): Client can't send new graphql queries after a while Oct 4, 2023
Copy link
Member

@TiagoJacobs TiagoJacobs left a comment

Choose a reason for hiding this comment

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

maybe instead of not breaking, you will want to avoid writing to the channel after the sessionToken is received?

@sonarcloud
Copy link

sonarcloud bot commented Oct 4, 2023

Kudos, SonarCloud Quality Gate passed!    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
0.0% 0.0% Duplication

@gustavotrott
Copy link
Collaborator Author

gustavotrott commented Oct 4, 2023

maybe instead of not breaking, you will want to avoid writing to the channel after the sessionToken is received?

Thanks for the suggestion, it was not possible to use sessionToken as condition at that point.
But as we talked later, I created a wrapper around the channel that uses a mutex to coordinate between sending and closing.
This way the reader is able to close the channel and the writer stops sending new msgs to it.
The code is much better now.

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

Automated tests Summary

🚨 Test workflow has failed


Click here to check the action test reports

@TiagoJacobs TiagoJacobs merged commit a2e2d05 into bigbluebutton:v3.0.x-release Oct 4, 2023
25 of 26 checks passed
@danimo danimo mentioned this pull request Nov 28, 2023
1 task
@gustavotrott gustavotrott deleted the graphql-losing-connection branch March 11, 2024 16:52
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.

None yet

2 participants