Include connectivity to server check in invalid server or credentials error #2545
Conversation
| @@ -86,7 +86,7 @@ func (u *UserDTO) CanPublish() bool { | |||
| return u.UserRole == AuthRoleAdmin || u.UserRole == AuthRolePublisher | |||
| } | |||
|
|
|||
| var errInvalidServerOrCredentials = errors.New("could not validate credentials; check server URL and API key or token") | |||
| var errInvalidServerOrCredentials = errors.New("could not validate credentials; check connectivity to the server, the URL, and API key") | |||
| var errInvalidApiKey = errors.New("could not log in with the provided credentials") | |||
| var errInvalidServer = errors.New("could not access the server; check the server URL and try again") | |||
There was a problem hiding this comment.
With a Server URL connectivity issue when creating credentials, I encountered this error. Did we not want to also update this message to check connectivity?
There was a problem hiding this comment.
Should we also update the second and third errors to show the server URL with which we're trying to connect?
There was a problem hiding this comment.
Did we not want to also update this message to check connectivity?
For this error it does include "could not access the server" which to me covers the case where the server is down. Were you thinking something different with the error?
There was a problem hiding this comment.
we also update the second and third errors to show the server URL with which we're trying to connect
Since those two are used during or right after inputing the URL I don't think it is necessary to repeat it like that. I could be more convinced on the second, but then I worry about error length.
There was a problem hiding this comment.
For this error it does include "could not access the server" which to me covers the case where the server is down. Were you thinking something different with the error?
Fair enough.
Since those two are used during or right after inputting the URL, I don't think it is necessary to repeat it like that. I could be more convinced on the second, but then I worry about error length.
You are correct in that connectivity to the URL has been verified when we get to this step. So when we fail here, the API key may no longer be valid or our validation of the server URL has issues. I think it would be useful to have the server URL present if I were trying to track it down (I think instant visual validation might help), and I'm more interested in providing the information to the user rather than the brevity of the error message. But I'm not strongly tied to this, I'll leave it to you to weigh the benefits and costs.
|
Verified! Marking issue as Done |
This PR changes the
errInvalidServerOrCredentialsto be more broad to include the case where the Connect server is down or cannot be connected to. Now rather than only suggesting to check the provided server URL and API key it now also suggested checking "connectivity to the server".Intent
Resolves #2450