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

Improve error message when a region is incorrect #439

Closed
jnioche opened this issue Feb 12, 2024 · 8 comments · Fixed by #447
Closed

Improve error message when a region is incorrect #439

jnioche opened this issue Feb 12, 2024 · 8 comments · Fixed by #447
Labels
enhancement New feature or request

Comments

@jnioche
Copy link
Collaborator

jnioche commented Feb 12, 2024

Problem

cargo run -- --aws-region eu-east-1 estimate --hours-use-time 10 | jq
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/cloud-scanner-cli --aws-region eu-east-1 estimate --hours-use-time 10`
cloud_scanner_cli: Using default API at:  https://api.boavizta.org
thread 'main' panicked at cloud-scanner-cli/src/aws_inventory.rs:91:14:
called `Result::unwrap()` on an `Err` value: Cannot list instances

Caused by:
    0: dispatch failure
    1: io error
    2: error trying to connect: dns error: failed to lookup address information: Name or service not known
    3: dns error: failed to lookup address information: Name or service not known
    4: failed to lookup address information: Name or service not known
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The problem is simply that eu-east-1 is a typo. us-east-1 was the correct value.
The error message could be more explicit and tell the user that there is no such region.

Solution

An internal lookup of the allowed values and a message indicating that the value does not exist.

@demeringo
Copy link
Collaborator

Hi @jnioche, thank you for reporting this issue.

I fully agree, error handling deserve better error messages.
For info, we have a similar issue when credentials are incorrects #164

@demeringo demeringo added the enhancement New feature or request label Feb 15, 2024
@demeringo
Copy link
Collaborator

@jnioche please just let me know if you intend to propose a PR for this, (otherwise I will put it on my todo list).

@jnioche
Copy link
Collaborator Author

jnioche commented Feb 15, 2024

@jnioche please just let me know if you intend to propose a PR for this, (otherwise I will put it on my todo list).

my RUST skills are very limited, I'm afraid.

@demeringo
Copy link
Collaborator

@jnioche please just let me know if you intend to propose a PR for this, (otherwise I will put it on my todo list).

my RUST skills are very limited, I'm afraid.

No worries with Rust ;-)

I much appreciate you reported the issue in the first place.

Until we have a proper fix, I will mention it in the documentation #442 .

@jnioche
Copy link
Collaborator Author

jnioche commented Feb 16, 2024

Had a quick look, there is no way of validating a region in the AWS SDK. There is a JSON file in this repo listing all the regions; it would need to be imported as a resource.

@jnioche
Copy link
Collaborator Author

jnioche commented Feb 22, 2024

Actually, since I had specified an AWS profile with
export AWS_PROFILE
and that the config for that profile has

region = us-east-1

shouldn't the region specified there override the default value?

Setting
export AWS_DEFAULT_REGION=us-east-1 does not have an impact either

@demeringo demeringo linked a pull request Feb 28, 2024 that will close this issue
@demeringo
Copy link
Collaborator

Thanks to your feedback, I simplified the code related to how cloud-scanners picks up the region.

See this PR #447.

Now, the logic is:

  1. by default, cloud scanner uses the region that is explicitly passed in parameter of the CLI (or in the http request).
  2. if no region is explicitly passed, it falls back to using the region of the user environment (either in the AWS profile or using the AWS_DEFAULT_REGION environment variable).

Furthermore I added a check (and hoppefully a more explicit error message) after this step so that we should be warned if the region is not supported by cloud scanner (like if AWS opens a new region or if in case of a typo in the region name).

@jnioche
Copy link
Collaborator Author

jnioche commented Feb 29, 2024

Thanks @demeringo, just tested your change, it works fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants