Identity should include the body of the bad response into the exception message

[antkmsft]

Somewhat real world scenario: an SDK developer did not set scope in the token request context.
Server returns 400 Bad Request. And the body says that the scope is empty which is the error.
But the exception being thrown just says "400 Bad Request". And you have to debug to see the actual error.

[LarryOsterman]

Does identity throw an exception based on RequestFailedException? If so, the what() message should include the error.Message field. If not, why doesn't identity throw an exception based on RequestFailedException?

[antkmsft]

Hmm, I am not sure if I understood your question, but these lines could explain it better:

azure-sdk-for-cpp/sdk/identity/azure-identity/src/token_credential_impl.cpp

Lines 105 to 125 in 7632d67

	response = m_httpPipeline.Send(request->HttpRequest, context);
	if (!response)
	{
	throw std::runtime_error("null response");
	}
	auto const statusCode = response->GetStatusCode();
	if (statusCode == HttpStatusCode::Ok)
	{
	break;
	}
	request = shouldRetry(statusCode, *response);
	if (request == nullptr)
	{
	throw std::runtime_error(
	std::string("error response: ")
	+ std::to_string(
	static_cast<std::underlying_type<decltype(statusCode)>::type>(statusCode))
	+ " " + response->GetReasonPhrase());
	}

That is the current code. Looks reasonable at first glance. But the problem is that ReasonPhrase is "Bad Request". It will never be more detailed that that, and it is not very helpful.

Meanwhile, the response body has good details, here is one of such responses:

{
    "error": "invalid_request",
    "error_description": "AADSTS90014: The required field 'scope' is missing from the credential. Ensure that you have all the necessary parameters for the login request. Trace ID: 0bd3b8d0-53d7-4e27-9d17-44555e721100 Correlation ID: a11856c7-1953-4246-b8e0-350cc749eed7 Timestamp: 2023-11-10 09:26:10Z",
    "error_codes": [
        90014
    ],
    "timestamp": "2023-11-10 09:26:10Z",
    "trace_id": "0bd3b8d0-53d7-4e27-9d17-44555e721100",
    "correlation_id": "a11856c7-1953-4246-b8e0-350cc749eed7",
    "error_uri": "https://login.microsoftonline.com/error?code=90014"
}

I.e. remember you tripped on this too? At one point you also did not set the scope, and for 10-15 minutes we were wondering, what could be wrong with the request that the identity sends, having only the "400 Bad Request". Logs will never show the response body.

So, this work item is to get the details from the response body, and include them into what() after the HTTP status code and the reason phrase.

[LarryOsterman]

Hmm. It appears that Identity uses a non-standard response body on errors. The Azure REST API guidelines describe a schema for error responses, but it doesn't appear that identity follows it.

If identity followed the Azure-wide pattern for errors, then simply throwing RFE would be sufficient. But apparently it isn't.

[antkmsft]

In either case, Identity should be throwing AuthenticationException, because otherwise, when user calls client.DoSomething() it will not be possible to distinguish between the exception thrown from the client/service vs an exception thrown from the authentication layer.
Second, AuthenticationException does not always come from a web request - AzureCLI credential throws it when a CLI tool returns error, plus there are some validations that are being done inside Identity, so at least if we add a Response field to the AuthenticationException, it could be null in some cases. I.e. it would be a RequestFailedException only in some cases, but not always.