-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[Identity] Update MI Response Handler in IdentityClient #36906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue where IdentityClient doesn't handle empty response bodies correctly when communicating with MSAL. It adds a new parseResponseBody method that gracefully handles JSON parsing failures and provides structured error information based on HTTP status codes.
Key Changes:
- Added
parseResponseBodymethod to handle empty/invalid response bodies and create error objects matching MSAL's expected format - Refactored response handling in
sendGetRequestAsyncandsendPostRequestAsyncto use the new parsing method - Added comprehensive test coverage for both server (5xx) and client (4xx) error scenarios with empty bodies
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| sdk/identity/identity/src/client/identityClient.ts | Adds parseResponseBody method to handle JSON parsing failures and generate structured error responses based on HTTP status codes |
| sdk/identity/identity/test/internal/node/managedIdentityCredential/msalMsiProvider.spec.ts | Adds two new test cases verifying error handling for empty response bodies in server and client error scenarios |
| sdk/identity/identity/CHANGELOG.md | Documents the bug fix for response format handling in empty response situations |
Packages impacted by this PR
@azure/identity
Issues associated with this PR
Describe the problem that is addressed by this PR
INetworkModule, theHttpClientwill parse response with no body usingparseBodyand return an object with an error message. OurIdentityClientimplementation returns an undefined body when response body is empty; thus raisingCannot read properties of undefined (reading 'expires_on')when MSAL handles the response. This PR aligns our behavior according to MSALparseBodyso that MSAL can surface the corresponding error. We also add additional logging for easier debugging when response body is emptyWhat are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
Are there test cases added in this PR? (If not, why?)
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists