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: do not fail on short connections #517

Merged
merged 3 commits into from Jan 10, 2020

Conversation

johnsudaar
Copy link
Member

@johnsudaar johnsudaar commented Jan 9, 2020

Fix #516

  • Do not reuse ssh.Signer, the connection to ssh agent might be broken
  • Do not retry the connection if the server closed the connection
  • Add --bind to db-tunnel to bind a custom command. This is helpful for docker setups where listening on localhost wont make it available on the docker0 interface

johnsudaar added 2 commits Jan 9, 2020
Fix #516

* Do not reuse ssh.Signer, the connection to ssh agent might be broken
* Do not retry the connection if the server closed the connection
* Add --bind to db-tunnel to bind a custom command. This is helpful for docker setups where listening on localhost wont make it available on the docker0 interface
@johnsudaar johnsudaar requested a review from Soulou Jan 9, 2020
@@ -198,6 +207,14 @@ func handleConnToTunnel(sshClient *ssh.Client, dbUrl *url.URL, sock net.Conn, er
fmt.Printf("End of connection [%d]\n", connID)
// Connection timeout
if err != nil && strings.Contains(err.Error(), "use of closed network") {

// If the connection has been closed by the CLIENT, we must stop here and return a nil error
// If the connection has been closed by the SERVER, we must return a errTimeout and retry the connection
Copy link
Member

@Soulou Soulou Jan 9, 2020

Choose a reason for hiding this comment

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

Isn't it legitimate that the server closes the connection sometimes?

Copy link
Member Author

@johnsudaar johnsudaar Jan 9, 2020

Choose a reason for hiding this comment

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

Not on this connection, if the server closed the conneciton, it will break the conneciton at line 191. Here it's the ssh conneciton that broke.

db/tunnel.go Outdated Show resolved Hide resolved
@johnsudaar johnsudaar requested a review from Soulou Jan 10, 2020
@Soulou
Copy link
Member

@Soulou Soulou commented Jan 10, 2020

LGTM

@Soulou Soulou merged commit e463b0b into master Jan 10, 2020
1 check passed
@Soulou Soulou deleted the fix/516/do_not_fail_on_short_lived_connections branch Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants