Fix retry handling, HttpClient ownership, and Bedrock credential edge cases#194
Open
InboraStudio wants to merge 1 commit into
Open
Fix retry handling, HttpClient ownership, and Bedrock credential edge cases#194InboraStudio wants to merge 1 commit into
InboraStudio wants to merge 1 commit into
Conversation
fix(client): always retry 429 on all target frameworks The previous #if !NETSTANDARD2_0_OR_GREATER guard silently excluded 429 from retry on .NET 8+ (and every other modern runtime) because the NETSTANDARD2_0_OR_GREATER symbol is defined on all .NET 5+ targets, not just netstandard. Remove the conditional and unconditionally retry 429. fix(client): guard HttpClient disposal to only dispose SDK-owned clients AnthropicClient and AnthropicClientWithRawResponse previously called HttpClient.Dispose() unconditionally, which would destroy a caller-supplied HttpClient (e.g., from IHttpClientFactory). Track OwnsHttpClient in ClientOptions and skip disposal for externally-supplied clients. fix(bedrock): use OrdinalIgnoreCase for Content-Type comparison StringComparison.CurrentCultureIgnoreCase can fail in Turkish/Azerbaijani locales where uppercase I maps to dotless i. HTTP MIME types are ASCII tokens and must be compared with ordinal (byte-level) semantics. fix(bedrock/signer): copy headers dict before mutating in AWSSigner AWSSigner.GetAuthorizationHeader was adding host/x-amz-date/ x-amz-security-token directly to the caller-supplied headers dictionary. If a caller reuses the dict across retries, these signing headers accumulate, causing duplicate canonical headers and signature mismatches. Now works on an internal copy. fix(bedrock/signer): remove redundant .Replace after ToHexStringLower Convert.ToHexStringLower never produces hyphens; the .Replace is dead code that misleads readers into thinking hyphens are possible. fix(credentials): add amortized file-lock pruning to CredentialsFileProvider The static s_fileLocks ConcurrentDictionary<string, SemaphoreSlim> was never pruned, causing a slow handle leak in long-running servers that rotate credential file paths. PruneFileLocks() now removes idle semaphores on each token-refresh cycle and the dictionary uses StringComparer.Ordinal. fix(credentials): clarify semaphore-release contract in TokenCache TryStartBackgroundRefresh held the semaphore across Task.Run scheduling. Added comments documenting the three release paths and ensured the outer catch releases on Task.Run failure (thread-pool exhaustion) with the same comment as the code already implemented.
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
1 participant
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.
This PR fixes a few reliability and resource-management issues across the client, Bedrock adapter, and credentials layer.
No public API changes were made and all existing tests continue to pass.
What i have change