Return the body of HttpOperationExceptions (to view the detail)

[mikeblakeuk]

Given I use an Azure Rest NuGet (e.g. Power BI)
When an HttpOperationException occurs
Then I want the body of the response

For example client.Datasets.DeleteDatasetByIdAsync("BAD", "Missing", "None");

Returns "Operation returned an invalid status code 'Unauthorized'" but using Fiddler I can see there is a more detailed error.

62 {"error":{"code":"UnknownError","pbi.error":{"code":"UnknownError","parameters":{},"details":[]}}} 0

This is also true for other requests that are 404 / 500

This is not just for Power BI

[koolraap]

I agree! :-) See #693 (comment)

[mikeblakeuk]

@stankovski thoughts?

[mikeblakeuk]

Example #2 (As Delete isn't finished yet so not a fair test)

[amarzavery]

If the swagger is modeled to deserialize the errors correctly, then the generated code will honor that.
Take a look at this spec:

• Definition of the Error
• Referencing it in the operation
• Another Example

[koolraap]

@amarzavery That's true. But if you're consuming an API you don't control you can't fix that easily. As I mentioned in #693 AutoRest is punishing the consumer for a producer's mistake; which runs against a common RESTful philopsophy: favour the consumer over the producer.

AFAIK it's not easy to specify the error type in Swashbuckle (you need a SchemaFilter to add the error type, and then (IIRC) an OperationFilter), which means you'll be seeing more of these requests :-)

[brodyberg]

@koolraap That philosophy is interesting but I can't find it on searching,
can you provide a link?

On Wed, Apr 13, 2016 at 1:58 PM, koolraap notifications@github.com wrote:

@amarzavery https://github.com/amarzavery That's true. But if you're
consuming an API you don't control you can't fix that easily. As I
mentioned in #693 #693 AutoRest
is punishing the consumer for a producer's mistake; which runs against a
common RESTful philopsophy: favour the consumer over the producer.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#931 (comment)

[devigned]

It would make sense to me to include common http information in an exception by default.

[koolraap]

@brodyberg I wrote the API design guidelines where I work, and favour the consumer is point #1 :-) I read a bunch of (conflicting) REST best practice guideines prior to that. It was mentioned fairly regularly. Here's an example http://www.restapitutorial.com/media/RESTful_Best_Practices-v1_1.pdf

[brentonw]

+1 having this same problem. The C# code generated by AutoRest looks something like this:

        // ...
        _httpResponse = await this.Client.HttpClient.SendAsync(_httpRequest, cancellationToken).ConfigureAwait(false);
        if (_shouldTrace)
        {
            ServiceClientTracing.ReceiveResponse(_invocationId, _httpResponse);
        }
        HttpStatusCode _statusCode = _httpResponse.StatusCode;
        cancellationToken.ThrowIfCancellationRequested();
        string _responseContent = null; // <-- not getting set
        if ((int)_statusCode != 200)
        {
            var ex = new HttpOperationException(string.Format("Operation returned an invalid status code '{0}'", _statusCode));
            ex.Request = new HttpRequestMessageWrapper(_httpRequest, _requestContent);
            ex.Response = new HttpResponseMessageWrapper(_httpResponse, _responseContent);
            if (_shouldTrace)
            {
                ServiceClientTracing.Error(_invocationId, ex);
            }
            _httpRequest.Dispose();
            if (_httpResponse != null)
            {
                _httpResponse.Dispose();
            }
            throw ex;
        }
        // ...

The HttpResposne is available to the caller, but after the response gets wrapped by HttpResponseMessageWrapper, the body is not available - it should be made available to the caller to manually extract anything they need to get to.

[mikeblakeuk]

Any word on this?

[OzBob]

If I could get the source to build I would pull request the change to:
https://github.com/Azure/autorest/blob/master/AutoRest/Generators/CSharp/CSharp/Templates/MethodTemplate.cshtml#L307

For "default:" non-200 responses:

else if ("string".Equals(responsePair.Value.Body.Name.toLower()))
{

@:@(Model.DefaultResponse.Body.Name) _errorBody = await _httpResponse.Content.ReadAsStringAsync().ConfigureAwait(false);

[fearthecowboy]

Howdy!

In our planning for driving towards a stable '1.0' release, I'm marking this issue as 'deferred' 💤 and we're going to review it during the post-1.0 planning cycle.

It's not to say that we're not going to work on it, or that this isn't not important, but at the moment, we're picking and choosing the stuff we must do before 1.0. 🏇 🏇 🏇

We'll make sure we pick this back up at that point. 🎉

[mikeblakeuk]

@fearthecowboy Any news on if/when this will be added?

[eMazeika]

Eagerly awaiting for this to be in the roadmap (1.1?)

[mikeblakeuk]

Looks like you can

 try
                    {
                        await client.Datasets.SetAllConnectionsAsync(_config.WorkspaceCollectionName, workspaceId,
                            dataSet.Id.ToString(),
                            new Dictionary<string, object> {{"connectionString", targetDatabase.ToConnectionString()}});
                    }
                    catch (Microsoft.Rest.HttpOperationException httpOperationException)
                    {
                        var phase = httpOperationException.Response.ReasonPhrase; 
//Bad Request
                        var content = httpOperationException.Response.Content; 
// {"error":{"code":"InvalidRequest","message":"Invalid dataset. This API can only be called on a DirectQuery dataset"}}
                        throw new  PowerBIException(content );
                    }

[mikeblakeuk]

Still not loving this :(