Fix broken 401 retry logic and wire forceRefresh through default auth provider#187
Open
jethibau-msft wants to merge 1 commit intomicrosoft:masterfrom
Open
Fix broken 401 retry logic and wire forceRefresh through default auth provider#187jethibau-msft wants to merge 1 commit intomicrosoft:masterfrom
jethibau-msft wants to merge 1 commit intomicrosoft:masterfrom
Conversation
Two bugs fixed:
1. Fetch.ts: The 401 retry loop used do {} while(false) with continue,
which in JavaScript exits the loop immediately (continue in do-while
jumps to the condition check, which is false, so the loop ends).
This meant no REST client ever retried on 401 despite the code
clearly intending to. Fixed by replacing with a proper while(true)
loop that breaks after one retry.
Also changed headers.append() to headers.set() to avoid duplicate
Authorization headers on retry.
2. Client.ts: The default authTokenProvider created by getClient()
ignored the forceRefresh parameter defined by the
IAuthorizationTokenProvider interface. The parameter is now forwarded
to SDK.getAccessToken(), enabling token refresh when the host
supports it.
Background: ADO services reject access tokens within ~5 minutes of
expiry. The IAuthorizationTokenProvider interface defines forceRefresh
for exactly this scenario, but it was never wired through. Combined
with the broken retry loop, extensions had no way to recover from
near-expiry token rejections.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
@microsoft-github-policy-service agree company="Microsoft" |
Contributor
|
@jethibau-msft the command you issued was incorrect. Please try again. Examples are: and |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two bugs prevent ADO web extensions from recovering when the host returns a near-expiry access token that gets rejected by ADO services.
Bug 1: 401 retry in
issueRequest()never actually retries (Fetch.ts)The retry logic uses
do {} while(false)withcontinue. In JavaScript,continueinsidedo-while(false)jumps to the condition check (false), which exits the loop immediately. The intended retry never executes.Bug 2: Default auth provider ignores
forceRefresh(Client.ts)The
IAuthorizationTokenProviderinterface definesgetAuthorizationHeader(forceRefresh?: boolean), but the default provider created bygetClient()declares the function with no parameters, soforceRefreshis silently ignored.Fix
Fetch.tsdo {} while(false)with a properwhile(true)loop thatbreaks after one retryheaders.set()instead ofheaders.append()to avoid duplicate Authorization headers on retryClient.tsforceRefreshin the default auth providergetAccessTokento accept the optional parameter (forwards harmlessly until the SDK adds support via companion issue)Impact
Every ADO web extension using
getClient()REST clients is affected — no extension can recover from a 401 despite the code clearly intending to support retry. Long-running extension sessions hit intermittent failures when tokens approach expiry with no recovery path.Companion PR
The
azure-devops-extension-sdkalso needsgetAccessToken(forceRefresh?)to complete the chain: https://github.com/microsoft/azure-devops-extension-sdk (issue/PR to follow).