Skip to content

Conversation

@NicolaiHoel
Copy link
Contributor

This PR fixes the refresh token logic, assuring old tokens doesn't accumulate in the header.

@sarahsa
Copy link
Contributor

sarahsa commented Jan 20, 2026

By removing the injection of HttpClient in the HeimdallApiClient, we are removing the functionality for customers to add their own clients (for instance setting up their own resilience pipelines). In addition the injection is optional, so it shouldn't cause any big issues.

Let's discuss 🤓

_tokenExpiresOn = accessTokenProvider.GetTokenExpiry();
foreach (var header in accessTokenProvider.GetAccessHeaders())
{
HttpClient.DefaultRequestHeaders.TryAddWithoutValidation(header.Key, header.Value);
Copy link
Contributor

@sarahsa sarahsa Jan 20, 2026

Choose a reason for hiding this comment

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

Instead of creating a separate auth handler and removing existing functionality as previously mentioned here, I would suggest the following:

Keep HeimdallApiHttpClient as is, and add a check that if HttpClient.DefaultRequestHeaders already contains the header.key, then remove it first and then add the KeyValue pair as we are already doing here HttpClient.DefaultRequestHeaders.TryAddWithoutValidation(header.Key, header.Value.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue here is that HttpClient.DefaultRequestHeaders is not a dictionary as one could expect, but rather a key with an associated array. That means that when we add a new value, it is appended instead of overwriting the existing value and after some time the header becomes to big, overflows and then the request fails.

@NicolaiHoel
Copy link
Contributor Author

Closing this PR, might come back to this solution later

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