strapi / rfcs

RFCs for Strapi future changes
68 stars 33 forks source link

[v4] Error RFC #34

Closed alexandrebodin closed 2 years ago

alexandrebodin commented 2 years ago

Error RFC

This PR introduces the API Error formats for v4.

You can read it here

Stun3R commented 2 years ago

Great jobs on this RFC! That's all I wanted to see about error format 🔥

Maybe the fact that we can't differentiate a property from a subProperty in the path can be annoying because it means that path[0] will be for property & path[1] for subProperty ? If so it would be great to have something that tell more explicitly if it's a property or sub-property 😉

I think that the use of codes is almost mandatory, at least from the SDK point of view since users will not display all raw errors and will prefer to process the error messages and choose what to display. Thanks error codes we can do it easily in order to add localisations & user friendly error management 😎

I'll say it again but it's needed: Congrats to the team, you did an amazing job ❤️

alexandrebodin commented 2 years ago

Great jobs on this RFC! That's all I wanted to see about error format 🔥

Maybe the fact that we can't differentiate a property from a subProperty in the path can be annoying because it means that path[0] will be for property & path[1] for subProperty ? If so it would be great to have something that tell more explicitly if it's a property or sub-property 😉

-> I'm Not sure to understand ? it's a path so it could be ['addresses', '0', 'city', 'name'] to say there as an error in that location of the input data

I think that the use of codes is almost mandatory, at least from the SDK point of view since users will not display all raw errors and will prefer to process the error messages and choose what to display. Thanks error codes we can do it easily in order to add localisations & user friendly error management 😎

-> I see your point but the names will be standard / constants so you could do the same with the name property. Code would add more flexibility for sure. I' just don't think we will have the time to document it correctly for release.

I'll say it again but it's needed: Congrats to the team, you did an amazing job ❤️

Stun3R commented 2 years ago

Great jobs on this RFC! That's all I wanted to see about error format 🔥 Maybe the fact that we can't differentiate a property from a subProperty in the path can be annoying because it means that path[0] will be for property & path[1] for subProperty ? If so it would be great to have something that tell more explicitly if it's a property or sub-property 😉

-> I'm Not sure to understand ? it's a path so it could be ['addresses', '0', 'city', 'name'] to say there as an error in that location of the input data

-> Ok so your example tell there is an error in addresses input, the first one has a problem with city name, right? If so maybe it can become tricky with the length of the path or complex form but it seems good tho (need to see more example to truly understand this path).

I think that the use of codes is almost mandatory, at least from the SDK point of view since users will not display all raw errors and will prefer to process the error messages and choose what to display. Thanks error codes we can do it easily in order to add localisations & user friendly error management 😎

-> I see your point but the names will be standard / constants so you could do the same with the name property. Code would add more flexibility for sure. I' just don't think we will have the time to document it correctly for release.

-> Clearly and I understand that a lot :) We can go with names, use codes too but tell to users it's not complete atm and implement them on the fly when they are all there 🤔

I'll say it again but it's needed: Congrats to the team, you did an amazing job ❤️

MattieBelt commented 2 years ago

Great RFC 🎉,

I think to correctly handle i18n for error message, for example for validation but also in the Admin Panel, we need an abstract way of providing detailed error info.

Something like this

{
  "error": {
    ...
    "name": "ValidationError",
    "meta": {
      "errors": [
        {
          "path": ["property", "subProperty"],
          "message": "property.subProperty min is 12",
          "name": "MinimumCharacterError",
          "details": {
            "min": 12
          }
        }
      ]
    }
  }
}

You probably already have thought about that as you had a similar "details" property at 'root' level of the error in the REST example.

stafyniaksacha commented 2 years ago

How we can manage message translations ? (If we have human-readable string, we may want our string to be translated)

alexandrebodin commented 2 years ago

As mentionned in the RFC we could introduce a code per validation error to make them translatable with a details to pass params like @MattieBelt shared.

This is not a goal for this first iteration of error formats that's why we try to keep it minimalist right now so we can build from that. In the first version you should only consider the format as something that will not break. The values should only be used as is so we can extend and make error more precise along the way. Once we reach a stable list of errors we will add the code to allow for i18N scenarios or more automation.