Skip to content

Destroy socket when there was an error on it #1975

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 1 commit into from
Feb 14, 2020
Merged

Conversation

Gobie
Copy link
Contributor

@Gobie Gobie commented Sep 16, 2019

When error happens on socket, potentially dead socket is kept open indefinitely by calling "connection.end()".
Similar issue is that it keeps socket open until long-running query is finished even though the connection was ended.

It has to wait for #1946 to fix tests

@brianc
Copy link
Owner

brianc commented Nov 11, 2019

I'll try to take a look at this tomorrow/wed - sorry for the delay. It looks reasonable but I need to be at a computer to run some tests with it.

@brianc
Copy link
Owner

brianc commented Nov 14, 2019

one of the tests is still failing here - do you need my help getting it to pass? You may just need to rebase w/ master.

@Gobie
Copy link
Contributor Author

Gobie commented Dec 4, 2019

Hey, after the fix of tests in #1946 it waits for brianc/node-pg-pool#131. With this fix it works fine.

I also added version for pg-native even though it doesn't have this problem, just to stay consistent.

When error happens on socket, potentially dead socket is kept open indefinitely by calling "connection.end()".
Similar issue is that it keeps socket open until long-running query is finished even though the connection was ended.
@Gobie
Copy link
Contributor Author

Gobie commented Jan 7, 2020

@brianc as brianc/node-pg-pool#131 was merged I rebased it and it looks ok.

@Gobie
Copy link
Contributor Author

Gobie commented Feb 14, 2020

ping @brianc

@brianc
Copy link
Owner

brianc commented Feb 14, 2020

Hey nice! This looks good! Sorry for missing your earlier comment, I was having some email configuration issues up until recently. Fixed now though. 😄

@brianc brianc merged commit 8231531 into brianc:master Feb 14, 2020
@Gobie Gobie deleted the patch-1 branch February 14, 2020 17: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.

3 participants