[Bug] MSAL is failing to parse an error response from the server

[Avery-Dunn]

Library version used

1.17.0

Java version

8

Scenario

Other - please specify

Is this a new or an existing app?

The app is in production, I haven't upgraded MSAL, but started seeing this issue

Issue description and reproduction steps

A JSON parser is used to extract error messages and other info from responses. However, it appears that some unexpected format is being sent for some errors, such as when there are too many requests:

We need to investigate why these responses aren't getting parsed, and ensure that error messages are correctly relayed from the server to the customer and MSAL provides provide meaningful messages.

Relevant code snippets

`com.microsoft.aad.msal4j.MsalClientException: com.fasterxml.jackson.core.JsonParseException: Unrecognized token 'Too': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')
at [Source: (String)"Too Many Requests"; line: 1, column: 4]`

Expected behavior

No response

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

No response

Solution and workarounds

No response

[jayendranar02]

Here are steps to investigate and potentially solve the issue:

Check Server Response:
Verify the exact response returned by the server when the error occurs. If it’s consistently returning plain text for certain error cases, this needs to be handled differently.

Modify Error Handling Logic:
Update the error handling logic in your MSAL implementation to check the content type of the response. If the content is plain text, handle it as such instead of attempting to parse it as JSON.

Implement Fallback Parsing:
Implement a fallback mechanism to handle unexpected response formats. If the parsing fails, catch the exception and log the raw response for further investigation.
Return Meaningful Messages:

Ensure that the error messages relayed to the client are meaningful. You might want to check for specific error codes or messages in the response and format them appropriately for the end user.

[jayendranar02]

try {
// Attempt to acquire a token or perform the operation
} catch (MsalClientException e) {
String errorMessage = e.getMessage();

// Check if the error message is plain text
if (errorMessage != null && errorMessage.equals("Too Many Requests")) {
    // Handle the specific case for "Too Many Requests"
    System.out.println("Rate limit exceeded. Please try again later.");
} else {
    // Attempt to parse as JSON
    try {
        // Assuming the response is accessible via e.getResponse()
        JsonNode jsonNode = objectMapper.readTree(e.getResponse());
        // Process the JSON error response as needed
    } catch (JsonParseException jsonEx) {
        // Log the raw error response
        System.out.println("Error parsing JSON: " + jsonEx.getMessage());
        System.out.println("Raw response: " + errorMessage); // This is plain text
    }
}

}

[jayendranar02]

Conclusion
By updating your error handling logic to account for unexpected formats, you can ensure that your application handles server responses more robustly. This will allow you to relay meaningful error messages to users, even when the server doesn't conform to the expected JSON format. Additionally, documenting any peculiar server behavior will help other developers understand how to handle such cases in the future.

[davidgiven]

I think I've just run into exactly this issue; the server is reporting a 503 with a non-JSON body, and the error handling in AadInstanceDiscoveryProvider.sendInstanceDiscoveryRequest() is throwing the wrong exception due to JSON parsing failing.

I believe this code should catch the JSON parse error and, in the event that it's not JSON, substitute a placeholder value and continue processing the HTTP error normally.