-
Notifications
You must be signed in to change notification settings - Fork 435
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
feat: add redirect info to connection errors #1527
Conversation
@@ -2055,7 +2055,14 @@ class Connection extends EventEmitter { | |||
* @private | |||
*/ | |||
connectTimeout() { | |||
const message = `Failed to connect to ${this.config.server}${this.config.options.port ? `:${this.config.options.port}` : `\\${this.config.options.instanceName}`} in ${this.config.options.connectTimeout}ms`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to directly create the message from the connection config host and port/instance. However, if there is a rerouting happened, we will be fail to connection to the rerouted host instead of the host from the connection config. It kind make sense in our case since we just let the user know that the host you indicated in the connection config is not connectable due to an error. Since we try to provide more detail on the routing server here, I think the message will be "fail to connect to the routing server (redirected from the orginal server) like the one in the change.
Codecov Report
@@ Coverage Diff @@
## master #1527 +/- ##
==========================================
+ Coverage 79.88% 80.60% +0.71%
==========================================
Files 92 92
Lines 4684 4692 +8
Branches 864 871 +7
==========================================
+ Hits 3742 3782 +40
+ Misses 672 630 -42
- Partials 270 280 +10
|
🎉 This PR is included in version 16.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
We got a request from a customer:
For errors connecting to the server a client is redirected to, those would show up as general connection errors. I can see the benefit of adding redirection server information to the message, though, when the client has been redirected. I’ll add it to our feature backlog for consideration in future semesters.
The redirection information will follow the server name in error messages. So for example, a connection timeout error to the redirected host after the initial host connection succeeds would look like:
com.microsoft.sqlserver.jdbc.SQLServerException: The TCP/IP connection to the host b35a36c6735e.tr32.westus1-a.worker.database.windows.net (redirected from yourtestdatabase.database.windows.net), port 11143 has failed. Error: "Connect timed out. Verify the connection properties. Make sure that an instance of SQL Server is running on the host and accepting TCP/IP connections at the port. Make sure that TCP connections to the port are not blocked by a firewall."
This PR added the redirect origin info to the connectionTimeout and socketError messages.