Ensure single use of HttpRequestMessage for token fetch #2544
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where HttpRequestMessage was being reused after failures in token fetch retries, which is not allowed. The request object initialization is moved inside the retry loop, and the error message is enhanced to include exception type information.
Key Changes
- Move
HttpRequestMessagecreation inside the retry loop to ensure a fresh instance per attempt - Enhance error message to include exception type alongside the message
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| for (int i = 0; i < maxRetries + 1; i++){ | ||
| try | ||
| { |
There was a problem hiding this comment.
[nitpick] Creating a new HttpRequestMessage on every retry attempt may be inefficient if the request creation involves expensive operations. However, this is necessary since HttpRequestMessage cannot be reused. Consider documenting this behavior with a comment explaining why the request must be created inside the loop.
| { | |
| { | |
| // HttpRequestMessage cannot be reused, so we must create a new one on each retry attempt. |
| { | ||
| throw new Exception("Failed to fetch token from server: " + e.Message); | ||
| throw new Exception( | ||
| $"Failed to fetch token from server: {e.GetType().FullName} - {e.Message}"); |
There was a problem hiding this comment.
Consider preserving the original exception's stack trace by using 'throw new Exception(..., e)' to include the inner exception. This would provide better diagnostic information while still customizing the error message.
| $"Failed to fetch token from server: {e.GetType().FullName} - {e.Message}"); | |
| $"Failed to fetch token from server: {e.GetType().FullName} - {e.Message}", e); |
rayokota
left a comment
rayokota
left a comment
There was a problem hiding this comment.
Thanks @Claimundefine , LGTM
|
What
HttpRequestMessage cannot be used multiple times after failure - Moving initialization inside loop and adding more descriptive error message.
Checklist
References
JIRA:
Test & Review
Open questions / Follow-ups