feat(auth): Add unit tests for auth failures in gax layer

[nakayamaryo0731]

Description

Fixes: #1350

I implemented the following two unit tests for CredentialError:

• auth_error_retryable() { ... }
• auth_error_non_retryable() { ... }

I tried to extend the existing retry_loop to implement the authentication loop process, but I couldn't get it to work properly. So, I implemented auth_retry_loop instead.

If you think it should be unified, please suggest an implementation approach.

Test

cargo test -p google-cloud-gax --test auth

[dbolduc]

/gcbrun

[dbolduc]

The tests look great! Thanks!

If you think it should be unified, please suggest an implementation approach.

I do think we need to unify the loops. Let's move the call to get the auth headers inside request_attempt(). I think that should achieve what we want and avoid duplication.

google-cloud-rust/src/gax/src/http_client/mod.rs

Line 177 in 5bf2f63

async fn request_attempt<O: serde::de::DeserializeOwned>(

Something like:

@@ -258,6 +242,14 @@ impl ReqwestClient {
         {
             builder = builder.timeout(timeout);
         }
+        let auth_headers = self
+            .cred
+            .get_headers()
+            .await
+            .map_err(Error::authentication)?;
+        for header in auth_headers.into_iter() {
+            builder = builder.header(header.0, header.1);
+        }
         let response = builder.send().await.map_err(Error::io)?;
         if !response.status().is_success() {
             return Self::to_http_error(response).await;

And we might want to run all of the tests for gax, to make sure we are not breaking existing retry loop tests:

cargo test -p google-cloud-gax

[dbolduc]

comment: As I suspected, this was a necessary change to use CredentialError outside of the google-cloud-auth crate. Thanks.

[nakayamaryo0731]

The tests look great! Thanks!

If you think it should be unified, please suggest an implementation approach.

I do think we need to unify the loops. Let's move the call to get the auth headers inside request_attempt(). I think that should achieve what we want and avoid duplication.

google-cloud-rust/src/gax/src/http_client/mod.rs

Line 177 in 5bf2f63

async fn request_attempt<O: serde::de::DeserializeOwned>(

Something like:

@@ -258,6 +242,14 @@ impl ReqwestClient {
         {
             builder = builder.timeout(timeout);
         }
+        let auth_headers = self
+            .cred
+            .get_headers()
+            .await
+            .map_err(Error::authentication)?;
+        for header in auth_headers.into_iter() {
+            builder = builder.header(header.0, header.1);
+        }
         let response = builder.send().await.map_err(Error::io)?;
         if !response.status().is_success() {
             return Self::to_http_error(response).await;

And we might want to run all of the tests for gax, to make sure we are not breaking existing retry loop tests:

cargo test -p google-cloud-gax

Thank you for the review!
I've applied your suggestions in the following commit:
f08f72f

I also confirmed that the GAX tests pass by running:

cargo test -p google-cloud-gax

[dbolduc]

/gcbrun

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.00%. Comparing base (5bf2f63) to head (f08f72f).
Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
src/gax/src/http_client/mod.rs	83.33%	1 Missing ⚠️
src/gax/src/retry_policy.rs	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1394      +/-   ##
==========================================
+ Coverage   95.93%   96.00%   +0.07%     
==========================================
  Files          37       37              
  Lines        1475     1477       +2     
==========================================
+ Hits         1415     1418       +3     
+ Misses         60       59       -1     

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

[dbolduc]

Looks great, thanks!