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

Displayed error when connecting to data controller is not always accurate #12365

Closed
Charles-Gagnon opened this issue Sep 16, 2020 · 2 comments · Fixed by #12573
Closed

Displayed error when connecting to data controller is not always accurate #12365

Charles-Gagnon opened this issue Sep 16, 2020 · 2 comments · Fixed by #12573

Comments

@Charles-Gagnon
Copy link
Contributor

Charles-Gagnon commented Sep 16, 2020

Got this error displayed - but the actual issue was that the "azdata login" command failed.

In this case it was because of a bad azdata build that didn't have the command. Should double check the logic though to make sure we're only displaying this error if we actually can't find azdata (versus the login command throwing an error)

The error message also shouldn't have the command embedded in it when called from outside the azdata extension since that's very specific to how the error is displayed.

AzureDataStudioError

@ranasaria
Copy link
Contributor

About why we got azdata missing error when it was a login issue:
Double-checked the code. We are throwing the error only when azdata is not found. One possibility is that this was invoked before discovery was complete. We can make the experience better by waiting for discovery to complete on first use. Right now, discovery not complete or azdata not found is just one state. Separating the two out will help us emit better errors.

About improving the error message. Here are a few points to consider:
a. if we had issued a toast the links would be visible.
b. if the issue had indeed happened due to the missing azdata then the second part of the message especially with the links would be useful.

So, I propose the following:

  1. Issue a different message not involving links since we do not want to use a toast here.
  2. Do the work to separate out the 'not yet discovered azdata installed, vs not installed'. Optionally we could wait for discovery to complete when the first request to use azdata comes in so that we do not fail the task due to incorrect sequencing reasons. Discovery, in general, won't take too long for it to be a perceptibly long wait.

@Charles-Gagnon
Copy link
Contributor Author

Let's start with #1 - any message which could be seen outside of the extension shouldn't have links embedded in it. Suggest making the base error not have links and then in the extension code where we want to display links catch errors and display a version with the links instead.

For the other let's keep an eye on it - seems like an edge case but we should try to make it less likely to be confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants