internal/registry: Add URL to error message for clarity

[radeksimko]

Prior to this patch the user could receive a message like this from init:

Error: Failed to install provider
Error while installing 1password/onepassword v1.1.2: could not query provider
registry for registry.terraform.io/1password/onepassword: failed to retrieve
authentication checksums for provider: the request failed after 2 attempts,
please try again later: 500

While the status code is helpful, it would be far more helpful if the user knew what URL/hostname returned that error.

I mistakenly thought this would come from the Registry API, but @bethanyr helpfully pointed out to me that the checksums are in fact pulled from GitHub.

The new (proposed) error message would therefore indicate this:

Error: Failed to install provider
Error while installing 1password/onepassword v1.1.2: could not query provider
registry for registry.terraform.io/1password/onepassword: failed to retrieve
authentication checksums for provider: the request failed after 2 attempts,
please try again later: 500 Internal Server Error (https://github.com/1Password/terraform-provider-onepassword/releases/download/v1.1.2/terraform-provider-onepassword_1.1.2_SHA256SUMS)

I am also open to shortening this to just the hostname (github.com in this case) instead of full URL if we deem that whole URL makes the message too long.

[alisdair]

This seems reasonable to me. Have you been able to verify what it looks like when there is such a failure? Does the UI really fail to wrap the error message?

[radeksimko]

I added some custom transports (locally) to simulate 500 and try this out which made realize that the place where I was fixing this was just responsible for the initial provider discovery, but the same client is apparently not responsible for downloading the checksums. I'm unsure what the reason is for the duplication, but I patched both places now.

Here is a screenshot from my testing

It's not amazing but I feel that the value of that message is greater with a full URL and this probably matters more?

[alisdair]

Thanks for adding this! Those screenshots look as good as we can hope for, and I agree that the URL will be helpful for debugging.

[radeksimko]

@alisdair would you like me to add this to the Changelog, or is it such a minor change for most users (which only ever hit the happy path) that it's not worth mentioning? If so - which Changelog (1.1.0 / 1.0.x)?

Relatedly - what's your policy on backports? I'm okay with not backporting this FWIW, although it's minor enough that it may be low effort to backport? 🤷🏻‍♂️

[alisdair]

I'd also skip the changelog, personally. It's not fixing a known issue, and it's not something anyone needs to be aware of in order to adjust their workflow.

Our policy for backports is loosely that only fixes for known issues or other critical changes go into patch releases. This doesn't seem like it's either of those, so I'd leave it on main for release with 1.1.0.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.