odata.net icon indicating copy to clipboard operation
odata.net copied to clipboard

Why ODataError.ErrorCode instead of ODataError.Code

Open Casimodo72 opened this issue 5 years ago • 4 comments

While reading the OASIS error response specification I noticed that the ODataError has a ErrorCode property instead of a Code property.

Does a way exist to serialize ODataError with JsonConvert.SerializeObject so that it spits out a "code" field instead of the "errorCode" field?

I would like my services to be compliant with the spec. How do you folks managed that? Did some of you implement your own ODataError (that's what I'm currently thinking of)?

Casimodo72 avatar Mar 09 '19 13:03 Casimodo72

Ok, found a way to make JsonConvert do what I need with a custom contract resolver.

EDITED: also added conversion of "innerError" to "innererror" and "StackTrace" to "trace".

public sealed class CustomODataErrorContractResolver : CamelCasePropertyNamesContractResolver
{
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        var prop = base.CreateProperty(member, memberSerialization);
        if (prop.PropertyName == "errorCode")
            prop.PropertyName = "code";
        else if (prop.PropertyName == "innerError")
            prop.PropertyName = "innererror";
       else if (prop.PropertyName == "stackTrace")
            prop.PropertyName = "trace";

        return prop;
    }
}
static readonly JsonSerializerSettings ODataErrorJsonSerializerSettings = new JsonSerializerSettings
{
    Formatting = Formatting.None,
    ContractResolver = new CustomODataErrorContractResolver()
};

Finally the ExceptionHandlerOptions for ASP Core's UseExceptionHandler:

public static ExceptionHandlerOptions GetCustomODataExceptionHandlerOptions(bool isDevelopment)
{
    return new ExceptionHandlerOptions()
    {
        ExceptionHandler = async (context) =>
        {
            var feature = context.Features.Get<IExceptionHandlerFeature>();

            context.Response.ContentType = "application/json";
            var response = new
            {
                error = ODataErrorHelper.CreateODataError(feature.Error, context.Response.StatusCode, isDevelopment)
            };

            await context.Response.WriteAsync(JsonConvert.SerializeObject(response, ODataErrorJsonSerializerSettings));
        }
    };
}

Casimodo72 avatar Mar 10 '19 08:03 Casimodo72

@Casimodo72 Thanks for reaching us. yes, it's better to use "Code" as property name, however, it's a breaking change currently. We'd suppose to do it in next major release.

xuzhg avatar Mar 13 '19 20:03 xuzhg

Please also note that the spec wants the ODataError property "InnerError" to be serialized as "innererror" (not "innerError") and the property "StackTrace" should be "trace". Non-breaking workaround: please consider adding a way to easily serialize an ODataError in compliance with the spec (e.g. dedicated contract resolver or even an extension method for JsonConvert).

It would also be great if people were made aware of this OData specification. I just fear that this will lead to lots of headaches w.r.t. interoperability with other OData services. Lots of programmers won't care/know and confuse/break error processing machineries. I didn't knew myself - until now.

Casimodo72 avatar Mar 29 '19 08:03 Casimodo72

ODataError.ToString() method outputs JSON that is not compliant with OData spec

Reproduce steps

The following code var error = new ODataError(){ Message = "Error" }; var json = error.ToString();

outputs the following JSON:

{"error":{"code":"","message":"Error","target":"","details":{},"innererror":{} }}

Note that empty "details" property is written as an object but according to OData spec (and ODataError class) it is an array. The ODataMessageReader fails to read such error payloads.

Expected result

Empty OData error details property is serialized as JSON array.

{
  "error": {
    "code": "error code",
    "message": "Unsupported functionality",
    "target": "query",
    "details": [
      {
        "code": "",
        "target": "target",
        "message": "message"
      }
    ],
    "innererror": {
      "trace": [],
      "context": {}
    }
  }
}

Actual result

What is actually happening.

{"error":{"code":"","message":"Error","target":"","details":{},"innererror":{} }}

Adding #1850 as a comment

marabooy avatar Aug 11 '20 17:08 marabooy

As part of the 8.0 investigation, consider the following differences between the ODataError type and what the standard requires:

  1. The standard requires that code be non-empty. ODL uses the empty string at least in cases where null is provided, and maybe in additional cases
  2. The standard requires that message be non-empty. ODL uses the empty string at least in cases where null is provided, and maybe in additional cases
  3. The standard allows target to be null, but ODL does not have a direct option to allow this.
  4. The standard requires that details be a JSON array, but when no details are provided, ODL uses {}
  5. The standard requires that the values within details be a complex type with properties code, message, and target, but ODL uses errorcode, message, and target
  6. The standard has the same non-empty requirements for code and message in details, but ODL has the same issues in this case as in ODataError
  7. The standard has the same null allowance for the target in details, but ODL has the same issues in this case as in ODataError
  8. The standard allows for complete flexibility within the innererror property, but ODL requires innererrors to have at least the same structure as ODataError

corranrogue9 avatar Nov 10 '22 02:11 corranrogue9