Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create ODataError for non-success responses #623

Merged
merged 17 commits into from
Jul 4, 2022

Conversation

KenitoInc
Copy link
Contributor

@KenitoInc KenitoInc commented Jun 20, 2022

When we return error messages from the controller e.g

BadRequest("Error Message");
Conflict("Error Message");
Unauthorized("Error Message");
NotFound("Error Message");
UnProcessableEntity("Error Message");

The Error Message is serialized as a string and the response is as follows:

{
  "@odata.context":"https://ServiceUri/odata/$metadata#Edm.String",
  "value":"Error Message"
}

In this PR, we add Custom methods in the ODataController that allows us to Create an ODataError from the Error Message hence the response will be Serialized as an Error to achieve the response below:

{
  "error": {
    "code": "400",
    "message": "Error Message"
  }
}

@KenitoInc KenitoInc marked this pull request as ready for review June 20, 2022 12:23
habbes
habbes previously approved these changes Jun 28, 2022
@KenitoInc KenitoInc dismissed stale reviews from habbes and xuzhg via 3dda6d0 July 4, 2022 08:21
Copy link
Contributor

@gathogojr gathogojr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@KenitoInc KenitoInc merged commit c4f229d into OData:main Jul 4, 2022

internal static void ValidateErrorCode(string expectedErrorCode, Microsoft.OData.ODataError error)
{
if (expectedErrorCode != error.ErrorCode)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

folks, this line looks like creating additional expectation that error.ErrorCode must be the same as HTTP status code, while according to the OData spec, it can be " language-independent string" (see below for more details)

9.4 Error Response Body
The representation of an error response body is format-specific. It consists at least of the following information:

· code: required non-null, non-empty, language-independent string. Its value is a service-defined error code. This code serves as a sub-status for the HTTP error code specified in the response.

· message: required non-null, non-empty, language-dependent, human-readable string describing the error. The Content-Language header MUST contain the language code from [RFC5646] corresponding to the language in which the value for message is written.

· target: optional nullable, potentially empty string indicating the target of the error, for example, the name of the property in error.

· details: optional, potentially empty collection of structured instances with code, message, and target following the rules above.

· innererror: optional structured instance with service-defined content.

Copy link

@garegina garegina Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is essentially a breaking change! it generates 500 internal server error for this code (suddenly the code that was valid before becomes invalid/not supported)

return NotFound(
new OData.ODataError()
{
ErrorCode = "ErrorEntityNotFound",
Message = "The specified entity was not found."
});

and may prevent clients from upgrading to the latest version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garegina You issue is valid. Fixed by #700

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants