catch exceptions when storing translations #391
Conversation
| $storage->create($message); | ||
| } catch (StorageException $e) { | ||
| throw new BadRequestHttpException(\sprintf('Key "%s" does already exist for "%s" on domain "%s".', $message->getKey(), $locale, $domain), $e); | ||
| } catch (\Exception $e) { |
There was a problem hiding this comment.
Is it really the behavior we're looking for? I'm afraid hiding all exceptions like this will make debugging much harder in case we have a problem in a storage isn't it?
There was a problem hiding this comment.
(this comment also applies for all others storage method calls (update & delete).
There was a problem hiding this comment.
Without the catch statement you see only Unknown error. in the UI. IMO this hides the actual problem even more.
|
Hi @Guite & thanks for your contribution. What's exactly the current behavior when an error occurred when interacting with the storage? |
|
Behaviour without this PR:
Behaviour with this PR:
|
|
I could also change it to |
The new message is great but it would be even better if we could keep the stacktrace! 🙏 |
|
Doesn't work unfortunately: it shows the entire stack trace inside the Bootstrap alert then. |
|
I don't know what to choose: with your changes, the end user message is way better and will ease resolving most of possible errors. On the other hand, we loose the stacktrace which can help a lot in some cases... |
|
My intention was that in production systems a user of the WebUI may experience problems which are not caused by programming errors. For example missing permissions like in the screenshots above or other server or config related issues. |
|
I like the new messages. But as @odolbeau points out, no solution is perfect. We should maybe look over this to see if there is a different way to solve this problem. Im happy with merging this for now. |
3 participants
This PR ensures that exception messages are returned with status code 400 so they are shown in the Web UI instead of
Unknown error..It also replaces the
400number by the correspondingResponseconstant.