Improve error messages when loading credentials. #1212
Conversation
This fixes googleapis#974. The error messages when loading an invalid or missing credentials file were sometimes very obscure (typically some parsing problem in the JSON library). The new error messages should make it easier to diagnose the problem.
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 92.2% 92.23% +0.03%
==========================================
Files 224 226 +2
Lines 12313 12370 +57
==========================================
+ Hits 11353 11410 +57
Misses 960 960
Continue to review full report at Codecov.
|
| char const REFRESH_TOKEN_KEY[] = "refresh_token"; | ||
| for (auto const& key : | ||
| {CLIENT_ID_KEY, CLIENT_SECRET_KEY, REFRESH_TOKEN_KEY}) { | ||
| if (credentials.count(key) == 0U) { |
There was a problem hiding this comment.
I feel like this new logic could be factored out into its own function, which would (probably) make testing it easier. If you had something like ParseAndValidateCredentialContent(), it could check if the content is valid/parseable JSON, check that the required fields exist, and make sure that those fields aren't empty (meaning we don't have to change the .get() calls below)... returning the nl::json object if all went well, or raising/dying at the first error it finds.
There was a problem hiding this comment.
Refactored, let me know what you think.
This fixes #974. The error messages when loading an invalid or missing
credentials file were sometimes very obscure (typically some parsing
problem in the JSON library). The new error messages should make it
easier to diagnose the problem.
This change is