Problem/Motivation

JSON:API error code should be string as stated in specification and json schema.

https://jsonapi.org/format/#errors

code: An application-specific error code, expressed as a string value.

Here is the provided example, which shows the code as a string and not an integer in the JSON response

HTTP/1.1 422 Unprocessable Entity
Content-Type: application/vnd.api+json

{
  "jsonapi": { "version": "1.0" },
  "errors": [
    {
      "code":   "123",
      "source": { "pointer": "/data/attributes/firstName" },
      "title":  "Value is too short",
      "detail": "First name must contain at least three characters."
    },
    {
      "code":   "225",
      "source": { "pointer": "/data/attributes/password" },
      "title": "Passwords must contain a letter, number, and punctuation character.",
      "detail": "The password provided is missing a punctuation character."
    },
    {
      "code":   "226",
      "source": { "pointer": "/data/attributes/password" },
      "title": "Password and password confirmation do not match."
    }
  ]
}

Proposed resolution

Cast the error code as string

Remaining tasks

Patch

User interface changes

N/A

API changes

N/A

Data model changes

Release notes snippet

JSON:API now provides HTTP error codes as strings instead of integers in order to comply with the specification. If your client application relies on an integer data type for an HTTP error code, it may need to be updated.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

el7cosmos created an issue. See original summary.

el7cosmos’s picture

Status: Active » Needs review
FileSize
818 bytes

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

mglaman’s picture

Status: Needs review » Needs work

Patch did not apply.

mglaman’s picture

Issue summary: View changes
mglaman’s picture

Issue summary: View changes
mglaman’s picture

Status: Needs work » Needs review
FileSize
1.71 KB

Here is an updated patch. I updated HttpExceptionNormalizerTest to use assertSame to verify we're getting strings. Otherwise, the test would consider (int) 13 eqaual to (string) 13.

Krzysztof Domański’s picture

Issue tags: +Needs change record

Looks good. Can we get a change record for the behaviour change to JSON:API Error Objects?

Krzysztof Domański’s picture

1. I add test only to make sure the result is integer.

2. Re-uploud previous patch, because RTBC patch should be last. I hope it will be RTBC soon.

3. The problem of this change is Backward Compatibility...

The last submitted patch, 9: 3086408-test-only-fail.patch, failed testing. View results

mglaman’s picture

#9, there isn't much to consider for backwards compatibility if we open a change record to announce the change. The only bug I could see is if someone tried to perform an integer-based operation on the code without running parseInt (JavaScript) or something similar in other frameworks.

mglaman’s picture

Gave a shot at a change record: https://www.drupal.org/node/3087598

Krzysztof Domański’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record

Thanks!

The last submitted patch, 9: 3086408-test-only-fail.patch, failed testing. View results

gabesullice’s picture

Issue tags: +API-First Initiative

Looks good to me! Thank you!

  • catch committed 56fd1b6 on 9.0.x
    Issue #3086408 by Krzysztof Domański, mglaman, el7cosmos: Error code...

  • catch committed 62d0824 on 8.9.x
    Issue #3086408 by Krzysztof Domański, mglaman, el7cosmos: Error code...

  • catch committed 4bd31c2 on 8.8.x
    Issue #3086408 by Krzysztof Domański, mglaman, el7cosmos: Error code...
catch’s picture

Version: 8.9.x-dev » 8.8.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.0.x and cherry-picked to 8.9.x and 8.8.x, thanks!

xjm’s picture

Issue tags: +8.8.0 release notes

Ah, this is another one of those minor changes to the exposed API for spec compliance. We'll add it to the release notes along with the other.

xjm’s picture

Issue summary: View changes

Updating the release note after checking with @gabesullice.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.