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.

Sign up for GitHub

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

Jump to bottom

fix: Handle socket timeout to prevent connections from leaking #407

Merged
merged 1 commit into from
Feb 20, 2023
Merged

fix: Handle socket timeout to prevent connections from leaking #407

merged 1 commit into from
Feb 20, 2023

Conversation

jirimoravcik
Copy link
Member

@jirimoravcik jirimoravcik commented Feb 16, 2023 •
edited

https://nodejs.org/api/net.html#event-timeout Emitted if the socket times out from inactivity. This is only to notify that the socket has been idle. The user must manually close the connection.

@jirimoravcik jirimoravcik added bug Something isn't working. medium priority Medium priority issues to be done in a couple of sprints. labels Feb 16, 2023
@github-actions github-actions bot added this to the 57th sprint - Platform team milestone Feb 16, 2023
@github-actions github-actions bot added the t-platform Issues with this label are in the ownership of the platform team. label Feb 16, 2023
fnesveda
fnesveda approved these changes Feb 17, 2023
View reviewed changes
Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

Nice! How did you find this?

@jirimoravcik jirimoravcik merged commit a460ad8 into master Feb 20, 2023
@jirimoravcik jirimoravcik deleted the fix/handle-socket-timeout branch February 20, 2023 10:01
@jirimoravcik
Copy link
Member Author

jirimoravcik commented Feb 20, 2023 •
edited

Nice! How did you find this?

netstat -pnat | grep 8000
https://serverfault.com/questions/738300/why-are-connections-in-fin-wait2-state-not-closed-by-the-linux-kernel

@kami4ka
Copy link

kami4ka commented Feb 22, 2023 •
edited

I thought it was my finding #362 (comment) 😂
Nice job, guys! Thanks for sharing this part of the functionality. The library is very cool for the domain and the community.

@jirimoravcik
Copy link
Member Author

I thought it was my finding #362 (comment) 😂 Nice job, guys! Thanks for sharing this part of the functionality. The library is very cool for the domain and the community.

Yeah, thanks for your comment. I started with my own investigations and then later found out you commented the same thing :D

@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fnesveda fnesveda fnesveda approved these changes

@drobnikj drobnikj Awaiting requested review from drobnikj

Assignees

@jirimoravcik jirimoravcik

Labels
bug Something isn't working. medium priority Medium priority issues to be done in a couple of sprints. t-platform Issues with this label are in the ownership of the platform team. validated Issues that are resolved and their solutions fulfill the acceptance criteria.
Projects
None yet
Milestone
57th sprint - Platform team
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jirimoravcik @kami4ka @fnesveda