-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Added actionable errors for network issues. #49246
Conversation
@rosstimothy Adding you to this because I added some timeouts in this PR when I realized no timeouts exists for Application Access. |
This pull request is automatically being deployed by Amplify Hosting (learn more). |
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.
I think this is a slight improvement, but what we've done here is taught the user how to recreate the same error outside of Teleport, not what they need to do to fix it.
For example, today users get confused when they see a connection refused error. After this change merges, they'll see an error that says "run nc -vz.." and when they run that they will see a very similar connection refused error to what they saw with Teleport. This doesn't seem like it will get the user much closer to understanding what's going wrong.
What do you think about making these error messages discuss the cause of the error more? For example, connection refused errors mean that packet(s) did reach the target host, but that nothing was listening on the port, whereas connection timeouts likely indicate some sort of firewall getting in the way.
What do you guys think about the following updated error messages?
|
I haven't encountered a firewall that results in a connection refused error. Usually they just silently drop packets and the client sees a network timeout. I'd move firewalls from here to the "context deadline exceeded" error message, otherwise these new messages look good. |
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.
Thank you @russjones!
24207f3
to
728d8b4
Compare
Added actionable errors for common network issues. Updated Application Access to use actionable errors.
5644115
to
6e53156
Compare
Added actionable errors for common network issues. Updated Application Access to use actionable errors.
Added actionable errors for common network issues. Updated Application Access to use actionable errors.
Added actionable errors for common network issues. Updated Application Access to use actionable errors.
Updated Application Access to return actionable error message when possible for network errors.
Fixes #47330