Skip to content

test(auth): add test for retry exhaustion on transient errors#8645

Draft
westarle wants to merge 1 commit into
googleapis:mainfrom
westarle:test-fixes/node-retry-exhaustion
Draft

test(auth): add test for retry exhaustion on transient errors#8645
westarle wants to merge 1 commit into
googleapis:mainfrom
westarle:test-fixes/node-retry-exhaustion

Conversation

@westarle

Copy link
Copy Markdown

Add retry exhaustion test to verify client request rejects with 500 error after all transporter retries are exhausted.

This brings Node.js test coverage in line with other resilient implementations (like Python, Go, Rust, and Swift) which also perform internal credential-level retries with exponential backoff on the token endpoint before ultimately exhausting and bubbling the error up. (Contrast this with Java, C++, and PHP which lack internal credential retries entirely and fail immediately on transient auth errors).

Add retry exhaustion test to verify client request rejects with 500 error after all transporter retries are exhausted.

This brings Node.js test coverage in line with other resilient implementations (like Python, Go, Rust, and Swift) which also perform internal credential-level retries with exponential backoff on the token endpoint before ultimately exhausting and bubbling the error up. (Contrast this with Java, C++, and PHP which lack internal credential retries entirely and fail immediately on transient auth errors).

TAG=agy
CONV=4aafb1cc-33ea-453d-968d-7e69305c0b3a

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a new unit test to test.oauth2.ts to verify that the client throws an error after retries are exhausted on transient errors. There are no review comments, and I have no feedback to provide.

},
})
.post('/token')
.times(4)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a constant or explicitly configured for this test? I think we are implicitly falling back to the default 3 retries in gaxios:

* The number of times to retry the request. Defaults to 3.

It looks like this library stores its config in this static method:

protected static get RETRY_CONFIG(): GaxiosOptions {

We could reference the config explictly like this:

.times(1 + OAuthClientAuthHandler.RETRY_CONFIG().retryConfig?.retry ?? 3)

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.

2 participants