[Cms] Refactor exceptions

[bastianallgeier]

In preparation for the UI translations, we should refactor all exceptions throughout the system, step by step.

I would like to replace all generic Exception throws with something more meaningful that we can translate out of the box.

We should take http://php.net/manual/de/spl.exceptions.php as foundation and create our own exceptions on top of it.

This is a huge task, so we need to split it down into multiple steps. We should start with the Roles classes, because they will need the most attention when it comes to translations.

Ideally we should end up with a scenario where each exception type has a default translation and on top of that we can provide custom translations if necessary.

It would also be great if we could make our exceptions a bit smarter when it comes to creating useful messages. I often use sprintf to format exception messages and it would be great to have that built in into the exception classes. We have to double check how this is possible, but ideally it would be something like this:

throw new InvalidArgumentException('The page slug "%s" is invalid', [$slug]);

We should also try to pass useful error codes, which might correspond with the matching HTTP status code in the API. Then we can automatically send meaningful HTTP response codes on errors in the API.

[distantnative]

How about?

throw new Exception([
  'key' => 'error.music.taste',
  'code' => 500,
  'fallback' => 'Listening to "%s" shows that your music taste sucks'
  'data' => [$song]
]);

throw new InvalidArgumentException([
  'key' => 'error.page.slug.invalid',
  'fallback' => 'The page slug "%s" is invalid'
  'data' => [$slug]
]);

or more traditional?

throw new Exception('error.page.slug.invalid', 'The page slug "%s" is invalid', [$slug], 500);

[distantnative]

On the other hand, do we really need a hardcoded fallback message, if we provide at least the english locales version anyways?

[lukasbestle]

If we don’t include a fallback message, it means that the locales need to be available at all times. What if someone uses a “Toolkit-y” class outside of the CMS, for example if we decide to publish some classes separately? Then we’d need to ship locales with those just for human-readable errors.

[distantnative]

True. But so far, I thought @bastianallgeier was just referring to the Kirby\Cms package.

[bastianallgeier]

No, there are more packages where we need this. Otherwise we have to extend everything within the Cms package that could throw an exception or make sure to catch all exceptions and pass them through to Cms variants, which would be a lot of hassle.

[distantnative]

Ok, then we really should hardcode a fallback message.

[bastianallgeier]

I was thinking that we provide a hard coded error message and translation code in each exception. That can be overwritten though if you want to be more specific while throwing the Exception.

Maybe it makes sense to have a Kirby\Exceptions package?

[lukasbestle]

What do you mean by “translation code”? Something like the Key in Nico’s example? I like his syntax a lot. Also I agree on a separate Exceptions package.

[distantnative]

First approach: https://github.com/k-next/kirby/pull/58

[bastianallgeier]

✅