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.

Sign up for GitHub

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

Jump to bottom

chore: improve dfx deploy message. #3967

Closed
wants to merge 4 commits into from
Closed

chore: improve dfx deploy message. #3967

wants to merge 4 commits into from

Conversation

vincent-dfinity
Copy link
Contributor

@vincent-dfinity vincent-dfinity commented Oct 29, 2024 •
edited
Loading

Description

Improve dfx deploy error message to let users know what to do next.

$ dfx deploy
Error: Failed to fetch the root key, did you run 'dfx start' to start the local replica?
An error happened during communication with the replica: error sending request for url (http://127.0.0.1:4943/api/v2/status)

Fixes # (issue)

SDK-1858

How Has This Been Tested?

Added an e2e test.

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@vincent-dfinity vincent-dfinity marked this pull request as ready for review October 29, 2024 14:36
@vincent-dfinity vincent-dfinity requested a review from a team as a code owner October 29, 2024 14:36
@marydwyer
Copy link

Is information about the rootkey at all relevent to the user? I'm wondering if we could streamline the message even further to something like this:

Error: Failed to deploy because your a replica is not running.
Please run 'dfx start' and try again.

@vincent-dfinity
Copy link
Contributor Author

Is information about the rootkey at all relevent to the user? I'm wondering if we could streamline the message even further to something like this:

Error: Failed to deploy because your a replica is not running. Please run 'dfx start' and try again.

  1. It just indicates where the error comes from.
  2. The reason why I use a question did you run 'dfx start' to start the local replica? is I'm not quite sure if it's the only reason to fail fetching the root key. @sesi200 do you know if there're any other cases?

ericswanson-dfinity
ericswanson-dfinity requested changes Nov 5, 2024
View reviewed changes
src/dfx/src/lib/root_key.rs
use dfx_core::network::root_key;

pub async fn fetch_root_key_if_needed(env: &dyn Environment) -> DfxResult {
let agent = env.get_agent();
let network = env.get_network_descriptor();
root_key::fetch_root_key_when_non_mainnet(agent, network).await?;
root_key::fetch_root_key_when_non_mainnet(agent, network).await
.map_err(|e| anyhow!("Failed to fetch the root key, did you run 'dfx start' to start the local replica?\n{}", e))?;
Copy link
Member

Choose a reason for hiding this comment

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

Please take a look at DiagnosedError. It may provide even more readable output, by first displaying the error, then displaying the explanation, and finally displaying a suggested action. You'd add it here with something like .context(DiagnosedError::new(...)).

@vincent-dfinity
Copy link
Contributor Author

I'll close this pr as this issue has been addressed by this pr #3973.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericswanson-dfinity ericswanson-dfinity ericswanson-dfinity requested changes

@sesi200 sesi200 Awaiting requested review from sesi200

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vincent-dfinity @marydwyer @ericswanson-dfinity