Skip to content

Add a callback for background token refresh outcomes#6099

Merged
neha-bhargava merged 1 commit into
nebharg/3682147-failure-metadatafrom
nebharg/3682729-background-refresh-callback
Jul 6, 2026
Merged

Add a callback for background token refresh outcomes#6099
neha-bhargava merged 1 commit into
nebharg/3682147-failure-metadatafrom
nebharg/3682729-background-refresh-callback

Conversation

@neha-bhargava

Copy link
Copy Markdown
Contributor

Proactive (background) token refresh is fire-and-forget: once the caller has a cached token, MSAL refreshes it on a background thread and the outcome is invisible from the outside. This adds an opt-in callback so that path can be observed - e.g. for telemetry parity during the SEAL -> MSAL migration.

  • New OnBackgroundTokenRefreshCompleted(Func<ExecutionResult, Task>) on the confidential client and managed identity builders (experimental).
  • On success the callback gets the AuthenticationResult; on failure it gets the exception, with the HTTP duration and cache-refresh reason on its AuthenticationResultMetadata.
  • Background refresh doesn't go through RunAsync, so the failure metadata is filled in from apiEvent in ProcessFetchInBackground.

Stacked on #6096 (which adds AuthenticationResultMetadata to MsalException) and targets that branch, so please review it after #6096. Work item 3682729.

Proactive (background) refresh is fire-and-forget, so callers can't see whether
it succeeded, how long it took, or why it failed. This adds an opt-in callback
(confidential client + managed identity) that hands back the AuthenticationResult
on success or the exception on failure - with the HTTP duration and cache-refresh
reason attached, so telemetry layers can report on this path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@neha-bhargava
neha-bhargava requested a review from a team as a code owner July 1, 2026 19:19
@neha-bhargava
neha-bhargava merged commit f87bc3e into nebharg/3682147-failure-metadata Jul 6, 2026
3 checks passed
@neha-bhargava
neha-bhargava deleted the nebharg/3682729-background-refresh-callback branch July 6, 2026 21:32
neha-bhargava added a commit that referenced this pull request Jul 8, 2026
* Surface token-acquisition failure metadata on MsalException

Adds a public AuthenticationResultMetadata property (getter public, setter
internal) to the base MsalException and populates it in RequestBase.RunAsync
for any in-request failure. Callers now get diagnostic metadata for failed
token attempts — total duration (always captured, useful for latency), plus
HTTP/cache durations, token endpoint, and region used when available —
mirroring the success-path AuthenticationResult.AuthenticationResultMetadata.

Only data actually captured is populated; RegionDetails is set only when a
region was used. TokenSource has no meaningful value on failure (documented).
The property is intentionally not part of the exception's JSON serialization.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Compute failure total-duration once for metadata and telemetry

Addresses PR review: the stopwatch keeps running, so re-reading it separately
for the exception metadata and the OTEL log produced slightly different totals.
Compute totalDurationInMs once in the catch and reuse it for both.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Populate RegionDetails on failure unconditionally, matching success path

Addresses PR review: the success path (UpdateTelemetry) always calls
CreateRegionDetails, so RegionDetails is non-null even when no region is used
(RegionOutcome.None). Drop the failure-path guard so both paths behave
identically. Updates the non-regional test to assert RegionOutcome.None.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add a callback for background token refresh outcomes (#6099)

Proactive (background) refresh is fire-and-forget, so callers can't see whether
it succeeded, how long it took, or why it failed. This adds an opt-in callback
(confidential client + managed identity) that hands back the AuthenticationResult
on success or the exception on failure - with the HTTP duration and cache-refresh
reason attached, so telemetry layers can report on this path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback on background-refresh callback contract

Skip the completion callback on proactive-refresh cancellation (benign - the caller already has a cached token), and wrap any non-MSAL failure in the general background path in MsalClientException so ExecutionResult.Exception is never null on failure. Also clarify the MsalException.AuthenticationResultMetadata doc: it is null only when the throw predates the request pipeline; when present, DurationTotalInMs is always populated. Adds a test asserting the callback is not invoked on cancellation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add test proving managed identity background refresh invokes the callback

bgavrilMS flagged the background path for managed identity. The callback is already wired for MI (ManagedIdentityApplicationBuilder.OnBackgroundTokenRefreshCompleted + the shared SilentRequestHelper.ProcessFetchInBackground that MI's proactive refresh uses); this adds the missing coverage that proves it fires for the MI flow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback: drop redundant metadata null-check, collapse dead if/else

In RequestBase.RunAsync the caught MsalException always has null AuthenticationResultMetadata on the foreground path (only this catch and the fire-and-forget background handlers set it), so the guard is redundant - assign unconditionally. Also collapse a pre-existing no-op if (ex.StatusCode == 400)/else in SilentRequestHelper (both branches logged identically) into a single call.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
neha-bhargava added a commit that referenced this pull request Jul 14, 2026
Add 4.86.0 changelog section (#6096 failure metadata, #6099 background-refresh callback, #6092 explicit-region IMDS skip, #6081 KeyGuard attestation fixes) and a backfilled 4.85.2 section (#6077 refresh-token cache partitioning, #6059 mTLS PoP min-strength), then move all unshipped public API entries to shipped across the six TFMs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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