Skip to content

Clear receive timeout when close browser and Deno channels #1016

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

Merged
merged 4 commits into from
Nov 18, 2022

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented Nov 14, 2022

The receive timeout was not being clearer when channel get closed.
With the timeout running, receive timeouts events can still be notified to the existing observers.
This was causing some tests failing and possible issues in production code.

This bug was found in the test package/neo4j-driver/test/result.test.js (should handle missing onCompleted). This test was calling done twice because of a late failing coming to the observer.

Node channels are not affected by this error since its timeout implementation is controlled to the socket.

@bigmontz bigmontz force-pushed the 5.x-remove-temporal-test-flakyness branch 3 times, most recently from 2730056 to 909e4c4 Compare November 17, 2022 09:29
The receive timeout was not being clearer when channel get closed.
With the timeout running, receive timeouts events can still be notified to the existing observers.
This was causing some tests failing and possibile issues in production code.
@bigmontz bigmontz force-pushed the 5.x-remove-temporal-test-flakyness branch from 909e4c4 to f00ca69 Compare November 17, 2022 10:25
@bigmontz bigmontz changed the title Remove random temporal types test flakynes Clear receive timeout when close browser and Deno channels Nov 17, 2022
@robsdedude robsdedude self-requested a review November 18, 2022 08:31
Copy link
Member

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

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

⏱️ 🔨

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