Expose error details in AMQ APIs #22114
Conversation
ghost
added
the
sdk/core/Azure.Core.Experimental/api/Azure.Core.Experimental.netstandard2.0.cs
Outdated
Show resolved
Hide resolved
| @@ -18,6 +18,15 @@ public enum ResponseStatusOption | |||
| } | |||
| namespace Azure.Core | |||
| { | |||
| public sealed partial class AzureError | |||
There was a problem hiding this comment.
Is this concept specific to Azure? i.e. should this be called ServiceError?
There was a problem hiding this comment.
It was called ServiceError in the first iteration. This exact shape is declared Azure REST guidenline as is Azure-specific.
There was a problem hiding this comment.
I'd prefer not to call it ServiceError because that'll cause a lot of confusion for anyone trying to search: https://docs.microsoft.com/en-us/dotnet/api/?term=serviceerror
There was a problem hiding this comment.
that'll cause a lot of confusion for anyone trying to search
Good point. Thought, I would still like to avoid using "Azure" in general purpose APIs. Maybe ServiceResponseError? or just ResponseError?
There was a problem hiding this comment.
docs.microsoft.com/en-us/dotnet/api/?term=serviceerror
I don't think there are that many search results. For example, https://docs.microsoft.com/en-us/dotnet/api/?term=Response didn't stop us.
There was a problem hiding this comment.
I kind of like either RequestError to tie it to RequestFailedException or ResponseError to tie it to Response*.
There was a problem hiding this comment.
RequestError sounds like it's a validation error (the request is wrong).
ErrorResponse?
There was a problem hiding this comment.
I think ErrorResponse implies that it's a subtype of Response.
There was a problem hiding this comment.
I like the ServiceError the most. It describes exactly what the type is about.
| { | ||
| get | ||
| { | ||
| _body.HasFailed = Status >= 400; |
There was a problem hiding this comment.
Would the value of HasFailed ever be false when the service operation succeeded? If so, why wouldn't we throw RequestFailedException rather than making someone interrogate this property? Is this just used for batches?
There was a problem hiding this comment.
It's for the batch response were some queries can succeed and some fail. For the single query scenario we always throw on failure.
sdk/monitor/Azure.Monitor.Query/src/Models/LogsBatchQueryResults.cs
Outdated
Show resolved
Hide resolved
…ts.cs Co-authored-by: Christopher Scott <chriscott@hotmail.com>
| { | ||
| internal ResponseError() { } | ||
| public string? Code { get { throw null; } } | ||
| public System.Collections.Generic.IReadOnlyList<Azure.Core.ResponseError> Details { get { throw null; } } |
There was a problem hiding this comment.
it's a bit strange that details of X is a collection of X.
There was a problem hiding this comment.
And it's even stranger that InnerError is a different type... but https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#7102-error-condition-responses
scottaddie
added
Monitor
Both batched and non-batched queries can partially succeed. Expose the error type and define it in Azure.Core.Experemental because it follows Azure-wide error definition from https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#errorresponse--object