fix(frontend): improve error handling for MaterialTable #3465
Conversation
I thought about generalizing this and include sentry reporting for each ServerError. I came to the conclusion that this is not very wise: Sometimes, server errors are part of the expected use case. Only the individual component can understand, if server errors are expected or not, so it should be the individual components responsibility to report unexpected server errors. Other opinions highly welcome. |
| @@ -11,7 +11,6 @@ | |||
| ref="quantity" | |||
| v-model="materialItem.quantity" | |||
| dense | |||
| vee-rules="numeric" | |||
There was a problem hiding this comment.
Is that no longer useful?
There was a problem hiding this comment.
Numeric only allows integer values. Our API allows float values. There's a built-in validation rule called "double" for float values. However, the "double" validation fails for empty values, which we explicitly allow.
Because we already set the html attribute type="number", the validation is not really needed, because the user cannot enter anything else than numbers anyway.
🤣 Bigger picture = Tried to fix some of the top-sentry issues on prod (on the API) from the last weeks and then went a bit down a rabbit hole when looking at how server errors are treated & displayed in the frontend. |
| @@ -50,7 +50,10 @@ public function normalize(mixed $object, string $format = null, array $context = | |||
|
|
|||
| $translations = $this->translateToAllLocalesService->translate( | |||
| $violation->getMessageTemplate(), | |||
| $violation->getParameters() | |||
| array_merge( | |||
| $violation->getPlural() ? ['%count%' => $violation->getPlural()] : [], | |||
There was a problem hiding this comment.
I did not see directly in which case you needed this.
A test which covers this path would be nice.
There was a problem hiding this comment.
Test added in f4baba3
Many symfony internal validators have violations messages that have a singular and a plural version. The %count% parameter needs to be provided to the translator such that it selects the right version:
https://github.com/symfony/symfony/blob/6.3/src/Symfony/Contracts/Translation/TranslatorInterface.php#L22
https://github.com/symfony/symfony/blob/6.3/src/Symfony/Component/Validator/Violation/ConstraintViolationBuilder.php#L123
Without this adjustment, the output would be the following (both singular+plural version returned):
'translations' => [
'en' => 'This value is too long. It should have 32 character or less.|This value is too long. It should have 32 characters or less.',
'de' => 'Diese Zeichenkette ist zu lang. Sie sollte höchstens 32 Zeichen haben.|Diese Zeichenkette ist zu lang. Sie sollte höchstens 32 Zeichen haben.',
'fr' => 'Cette chaîne est trop longue. Elle doit avoir au maximum 32 caractère.|Cette chaîne est trop longue. Elle doit avoir au maximum 32 caractères.',
'it' => 'Questo valore è troppo lungo. Dovrebbe essere al massimo di 32 carattere.|Questo valore è troppo lungo. Dovrebbe essere al massimo di 32 caratteri.',
],
Also fixes #3349:
HttpExceptionare now not excluded blindly anymore, but only if they equal to specific HTTP codes (excluded_http_codes). At the moment this is configured to exclude the following codes:400 Bad Request
401 Unauthorized
403 Forbidden
404 Not Found
405 Method Not Allowed
422 Unprocessable Entity
However, this works only for exceptions of type
HttpException. There are other exceptions that eventually respond with a 4xx status code. These exceptions still need to be excluded manually withignore_exceptionsBefore
After
The following screenshot is only intended to visualize the change in error display and translation. This would not be reproducible anymore after this PR, because input length is already limited to 64 characters.
