Implement State-Based Timeout Classification for Silent Token Requests, Fixes AB#3495207

[Prvnkmr337]

Fixes AB#3495207

Problem:
Generic TIMED_OUT errors provide no insight into timeout root cause, making production debugging difficult.

Solution:
Track request lifecycle state using correlation ID to classify timeouts.

CommandDispatcher.java:

• Add TimeoutTrackingState enum (WAITING_FOR_LOCK, QUEUED, EXECUTING)
• Add sTimeoutLocationMap for correlation ID → state tracking
• Add createTimeoutException() for state-based classification
• Track state transitions in submitSilentReturningFuture()
• Classify and cleanup in submitAcquireTokenSilentSync()

ClientException.java:

• Add TIMED_OUT_LOCK_CONTENTION
• Add TIMED_OUT_THREAD_POOL_SATURATED
• Add TIMED_OUT_THREAD_POOL_CONTENTION
• Add TIMED_OUT_EXECUTION

CommandDispatcherTest.java:

• Add 6 tests for timeout classification and state tracking
• Add helper methods and test utilities

[github-actions]

✅ Work item link check complete. Description contains link AB#3495207 to an Azure Boards work item.

[github-actions]

❌ Invalid work item number: AB#3495207

Problem:
Generic. Work item number must be a valid integer.

Click here to learn more.

[Copilot]

Pull request overview

This pull request implements state-based timeout classification for silent token requests to improve production debugging by providing detailed timeout root cause information. The solution tracks request lifecycle states (WAITING_FOR_LOCK, QUEUED, EXECUTING) using correlation IDs and classifies timeouts with specific error codes.

Changes:

• Added four new timeout error code constants to ClientException.java for precise timeout classification
• Enhanced CommandDispatcher.java with RequestState enum, state tracking map, and classification logic
• Added comprehensive test suite with 6 new tests covering various timeout scenarios and cleanup verification

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
ClientException.java	Added four new timeout error codes (TIMED_OUT_LOCK_CONTENTION, TIMED_OUT_THREAD_POOL_SATURATED, TIMED_OUT_THREAD_POOL_CONTENTION, TIMED_OUT_EXECUTION) with clear documentation
CommandDispatcher.java	Implemented RequestState enum, sTimeoutLocationMap for tracking, state transitions in submitSilentReturningFuture, and createTimeoutException for classification
CommandDispatcherTest.java	Added 6 tests for timeout classification scenarios, state collision prevention, cleanup verification, and correlation ID propagation
changelog.txt	Added entry documenting this feature as a MINOR change

Comments suppressed due to low confidence (1)

common4j/src/main/com/microsoft/identity/common/java/controllers/CommandDispatcher.java:478

• Memory leak for cached commands: When a command is eligible for caching and already executing, this code returns the existing future immediately (lines 474, 478). However:

1. The state was already set to WAITING_FOR_LOCK (line 454) and then QUEUED (line 458) for the second request
2. The function returns early without scheduling a new runnable
3. The state entry for the second request's correlation ID is never cleaned up because:
   • The runnable's finally (line 528) only removes the first request's correlation ID
   • submitAcquireTokenSilentSync's finally (line 402) only removes if that specific method was called

Impact: Every duplicate cached request leaks one entry in sTimeoutLocationMap, causing unbounded memory growth for frequently repeated requests.

Recommendation: Before returning early, remove the correlation ID from sTimeoutLocationMap:

sTimeoutLocationMap.remove(correlationId);
return putValue; // or future

Apply this at both early return points (lines 474 and 478).

            if (command.isEligibleForCaching()) {
                FinalizableResultFuture<CommandResult> future = sExecutingCommandMap.get(command);

                if (null == future) {
                    future = new FinalizableResultFuture<>();
                    final FinalizableResultFuture<CommandResult> putValue = sExecutingCommandMap.putIfAbsent(command, future);

                    if (null == putValue) {
                        // our value was inserted.
                        future.whenComplete(getCommandResultConsumer(command));
                    } else {
                        // Our value was not inserted, grab the one that was and hang a new listener off it
                        putValue.whenComplete(getCommandResultConsumer(command));
                        return putValue;
                    }
                } else {
                    future.whenComplete(getCommandResultConsumer(command));
                    return future;