Exception handling and SS_UserException
Sam Minnee
what people thought about this idea:
Should we make class SS_UserException extends Exception, designed to
be used to throw exceptions where the message on that exception is
acceptable to be sent back to end-users?
It would mean that you could do things like this in a form action
handler, without risking that sensitive diagnostic information for
developers is sent back to end users:
function doMyAction($data, $form) {
try {
// something
} catch(SS_UserException $e) {
$form->sessionMessage($e->getMessage(), "error");
$this->redirectBack();
}
// redirect to the success page;
}
What do people think?
dalesaurus
back to the user that are not necessarily PHP errors. I make good use
of the Forms feature to throw Session Messages of good/bad/warn when
possible, and think that is a good starting model. Having a way to
send "flash" messages back in ajax scenarios would also be great.
One item to consider is baking group security levels in this system.
At the very minimum the ability to message an Admin differently than
Registerd Users vs Anonymous Users would be useful.
Also if discussing a move towards exception based coding would it make
sense to have a way to consistently wrap method calls in try{} blocks
via magic methods?
Hamish Campbell
unforseen or unexpected (...exceptional) occurs. While I think that
better Exception handling would be a good feature in 3.0, I don't
think that routinely using Exceptions to report messages to the user
is a good idea. In the case of form validation, receiving non valid
form data is an entirely expected result so handling with an Exception
is inappropriate.
That is, if an Exception occurs, there is something that needs to be
fixed.
I've also read (but haven't confirmed this) that using a try..catch
block to, for example, catch a form validation exception is
considerably less performant than simply returning a validation
result.
Hamish Campbell
Sam Minnée
> An exception is supposed to occur when something
> unforseen or unexpected (...exceptional) occurs.
> That is, if an Exception occurs, there is something that needs to be
> fixed.
These are two different statements. To be honest, I don't know whether user-input error counts as the flow-control-via-exceptions anti-pattern. But your second statement is very restrictive - it suggests that a 404s (indeed any 400-level error) wouldn't be allowed to be handled via exceptions. I've done a bit of googling and can't find any specific guidance. When people are raise the anti-pattern it's usually about much more banal stuff, like using exceptions to handle inappropriate passing of nulls.
> I've also read (but haven't confirmed this) that using a try..catch
> block to, for example, catch a form validation exception is
> considerably less performant than simply returning a validation
> result.
OK I did a little benchmark: https://gist.github.com/848044
The output was as follows.
Time without exception: 0.44 microseconds / call
Time with try but never throwing exceptions: 0.48 microseconds / call
Time throwing exceptions half the time: 7.08 microseconds / call
Time throwing exceptions all the time: 14.51 microseconds / call
So, yes, it looks like we'll be adding 14 microseconds to a form processing request. I think I can live with that. ;-)
(For comparison, simply constructing an Exception object hand not throwing it takes about 4.4 microseconds.)
The key is that we'll only be throwing & catch a single exception for a request, whereas the really bad exception anti-patterns might use Exceptions 100s or 1000s of times within a single request, which would start to bloat out code.
Hamish Friedlander
--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To post to this group, send email to silverst...@googlegroups.com.
To unsubscribe from this group, send email to silverstripe-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.
Sam Minnée
There are two ways in which we could allow for "user-readable exceptions":
1) SS_UserException - which has a single message
2) SS_Exception - which lets you define two messages as two separate parameters:
- message
- userMessage (defaulting to something innocuous, like "Sorry, something went wrong.")
The advantage of the SS_UserException class is that you can easily trap *only* those exceptions that have user-readable messages, and, for example, show them in the error message part of a form.
However, upon further reflection (and discussion with Hamish F), I'm not sure that this is such a problem. If, for example, there was a database error on a form submission, it would make sense to put "Sorry, we couldn't talk to our database." in the form's error message, rather than showing a generic error page.
That said, it may make logging difficult - we would want to log some exceptions but not others. Exceptions generated by user actions needn't be logged. Right now, the best way of logging an exception is to leave it uncaught, but that's a little crude.
Perhaps we need a separate class, not for user *readable* exceptions, but for user *caused* exceptions. Something, SS_UserGeneratedException or SS_UnloggedException.
Coming full-circle, instead of SS_UnloggedException we could have an $isUserGenerated flag on SS_Exception, and let the logging code inspect that when deciding whether or not to log a message.