Skip to content

Conversation

@tsibley
Copy link
Contributor

@tsibley tsibley commented Jan 27, 2024

See commits. Addresses review feedback for #333.

Checklist

  • Checks pass

@victorlin pointed out¹ that a UserError is not raised by this function.

While correcting that, I noted that other docstrings in this module use
"saved credentials" as the term (matching the term we use in
user-visible output) instead of "tokens", so I made this docstring
follow suit.

¹ <#333 (comment)>
@victorlin suggested¹ we improve error handling in this code path, and
he's right.

Catch and improve the output for the three most common errors we'll
encounter: connection errors, HTTP errors, and JSON decoding errors.
The original error is summarized in the error output, and by chaining
the new exception from the original, the full chain in all its detail
will be printed when NEXTSTRAIN_DEBUG=1 for troubleshooting.

¹ <#333 (comment)>
@tsibley tsibley requested review from a team and victorlin January 27, 2024 00:22
Avoids needlessly trying to fetch authn metadata from the remote origin,
which leads to nicer UX.  For example, previously `nextstrain whoami
bogus` resulted in a connection error re: fetching authn metadata but
now it results in a more sensible "not logged in" error.
@tsibley tsibley merged commit 2b0b4ba into master Jan 29, 2024
@tsibley tsibley deleted the trs/remotes-review-feedback branch January 29, 2024 17:43
@tsibley
Copy link
Contributor Author

tsibley commented Jan 29, 2024

Releasing this as 8.0.1.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants