enhancement: make policy error public

[innerdvations]

What does it do?

PolicyError messages will be public to the API as originally intended

Why is it needed?

PolicyError was created to allow policies to throw errors with messages visible to the user. Unfortunately, it was broken for all (or nearly all) of v4, so it was considered a security risk to suddenly make them visible at that point.

In v5, we will add the originally itended behavior to allow policies to throw a custom error message.

How to test it?

Create a policy that throws a custom error message and you should see it
Any other type of ForbiddenError should still be caught and made generic (ie, the message should not be included in the api response)

Related issue(s)/PR(s)

Let us know if this is related to any issue/pull request

[derrickmehaffy]

The docs on this state that technically any error can be used in a policy and that error type should be used 🤔

https://docs.strapi.io/dev-docs/error-handling#policies

At least for all of our built in error classes. (I'm thinking things like ratelimit errors)

[innerdvations]

The docs on this state that technically any error can be used in a policy and that error type should be used 🤔

docs.strapi.io/dev-docs/error-handling#policies

At least for all of our built in error classes. (I'm thinking things like ratelimit errors)

Hmm. It does seem to say that, but I don't think that has ever worked that way, has it? We might need to update the v4 docs again to clarify.

I think this might be the best we get for v5 RC, but I would LOVE to improve on it and somehow catch anything coming from a policy and make it public. I was thinking an alternative (that could extend beyond policies and into all of Strapi) is that an error could simply have a public property and if it does, we let it through to the content API. But that can come later in v5 as a new feature and not break anything.

EDIT: Ohhhh, but reading what you said again, yeah, this brings it in line with all the other "special" error types being public. RateLimit, etc. The only ones that are converted to generic are ForbiddenError and ApplicationError I believe.

[derrickmehaffy]

It did originally yeah as I did a whole best practices talk about this a while back.

[derrickmehaffy]

Yeah those extra classes just build on the generic application error usually.

[derrickmehaffy]

Here is the talk I did: https://youtu.be/dGNVYTEJNY8?si=2bArhx8_ncghQidJ

[derrickmehaffy]

Also FYI I'm the one who wrote those docs and created that policy error if you look at the git blame 🙈

[derrickmehaffy]

#12080

#15938

[innerdvations]

Yeah, so to be clear, I think this is actually all we need to do, right? Everything except forbiddenError and unauthorizedError are thrown to the content API. And PolicyError is only "special" because it's the only one that inherits from ForbiddenError so we have to watch out for it explicitly.

I suppose a better option would be to start with the list of "allowed" ones and find/throw them first, I might update to do it that way so it's more clear about what we're doing and it can't get messed up again in the future.

(Edit) Let's discuss this today before I do anything, we might be able to simplify all of this and go with what was originally intended when you wrote the feature

[christiancp100]

QAed it and works great! 🍝