fix(cli): cdk notices ignores failures and calling cli with --no-notices makes an unexpected network call

[mrgrain]

Fixes #370
Fixes #440

Previously when the --no-notices option was passed, the CLI would still attempt to refresh notices in the background. This is unnecessary and an unexpected behavior to users, especially when running the CLI in environments with no internet access.

On the other hand, running the explicit cdk notices command did previously not fail if the command failed to refresh notices. Instead the command would pretend no notices are found and complete successfully.

This PR updates the code to not refresh notices when notices are disabled. Unless the explicit cdk notices command is called, which will always refresh and now error if fetching notices fails. It also removes a double refresh of the notices (which didn't have an effect because they were cached, but still).

cdk notices with an error now looks like this:

Before the command would incorrectly and silently succeed and not show any notices:

Only when run with the -v flag, it would hint at the failure:

$ cdk notices -v
...
[12:38:29] Could not refresh notices: Error: getaddrinfo ENOTFOUND cli.cdk.dev-tools

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 42.85714% with 12 lines in your changes missing coverage. Please review.

Project coverage is 79.17%. Comparing base (bd4ad74) to head (d8107f7).

Files with missing lines	Patch %	Lines
packages/aws-cdk/lib/cli/cli.ts	40.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #399      +/-   ##
==========================================
- Coverage   79.25%   79.17%   -0.08%     
==========================================
  Files          54       54              
  Lines        6897     6910      +13     
  Branches      772      773       +1     
==========================================
+ Hits         5466     5471       +5     
- Misses       1413     1421       +8     
  Partials       18       18              

Flag	Coverage Δ
suite.unit	79.17% <42.85%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rix0rrr]

This is unexpected to users and prevents the CLI from running in environments with no internet access.

If this prevents the CLI from running in those environments we have even more problems, because the behavior should be that failures get swallowed. This is not critical but best-effort behavior, so it's fine.

It's just for cdk notices that a fetch failure should be fatal.

[mrgrain]

This is unexpected to users and prevents the CLI from running in environments with no internet access.

If this prevents the CLI from running in those environments we have even more problems, because the behavior should be that failures get swallowed. This is not critical but best-effort behavior, so it's fine.

It's just for cdk notices that a fetch failure should be fatal.

Ah yes, you are correct. CachedDataSource makes the call non-fatal. I had only looked at the WebsiteNoticeDataSource code. But that means it's also not fatal for cdk notices.

[rix0rrr]

But that means it's also not fatal for cdk notices.

We could make a distinction in the internal state between "successful fetch with no notices" and "no fetch", and do something like:

notices.assertLastFetchSuccessful();

In the handling of cdk notices.

[mrgrain]

In this file I mostly make the error messages consistent. Variations of Failed to fetch notices are moved to one level up. Any errors from this file will show up as the cause printed in yellow.

[mrgrain]

AI generated. Needed an easy way to make errors more human.

[mrgrain]

refresh now throws

[mrgrain]

Unrelated bug, but previously in dev mode we would print the stack trace of the parent error twice.