Use of user_error() instead of exceptions.

67 views
Skip to first unread message

Patrick Nelson

unread,
Apr 6, 2016, 8:07:38 PM4/6/16
to silverst...@googlegroups.com
I wanted to throw in flame words like "pervasive" or "overuse" just to stir the pot a bit, but I want to express ahead of time that I'm assuming there's a good reason for this. On to my point: Is there some particular reason why SilverStripe prefers to use user_error() instead of throwing exceptions?

My assumption (benefit of the doubt) is that this is to ensure that execution can still continue without halting per se. Instead, the standard practice would be to simply return some default/empty value from the current method to prevent further execution locally without propagating that too far. 

However, this gets in the way of my ability to properly (or cleanly, I guess) handle errors when they occur. What if we just did a better job with exception handling and ensured we had a more solid process of wrapping particularly error prone operations with try{}/catch() blocks? See here for what you end up having to do in order to compensate for the fact that errors may occur but may not be catchable: https://github.com/patricknelson/silverstripe-migrations/blob/master/code/MigrateTask.php#L74

One solution to this may be to use something akin to "withExceptions()" wrapper which allows more programmatic control and capture over the errors that are triggered in SilverStripe. I have written this into my own adaptation of the SilverStripe framework so I can have fine grained control over any possible unexpected errors which may occur during various automated tasks (such as cron or deployment). It looks a bit like this, as a slight modification of the above, and uses closures in order to help isolate and maintain state properly: 


Maybe others will find this useful. What do you guys think? Could this be integrated more into SilverStripe at large or should it be just a module? Furthermore, should SilverStripe do more to use Exceptions either exclusively (with heavy use of try/catch blocks) or optionally as a generic placeholder (abstraction) via configuration in place of straight up use of the user_error() function?

- Patrick

Hamish Friedlander

unread,
Apr 6, 2016, 8:14:54 PM4/6/16
to silverst...@googlegroups.com
It's not so much that we prefer it, and more that we don't want a mix. Like tabs versus spaces, consistency is more important than being right :).

Last time we talked about changing over, we ended up in a (a little bit bike-shedding) discussion about what the Exception class hierarchy would look like. Converting all user_errors to just a few generic Exception subclasses don't give you the most powerful feature of exceptions (selectively handling the errors you know how to recover from, ignoring the rest).

There's actually code a bit like your gist already in SilverStripe occasionally - see https://github.com/silverstripe/silverstripe-framework/blob/3/model/fieldtypes/HTMLText.php#L101 - so extracting it to a utility function might be useful.

Designing a proper hierarchy probably needs to wait until after we've added namespaces consistently (which we're hoping to do in 4). So, 5 maybe?

Hamish Friedlander

Patrick Nelson

unread,
Apr 6, 2016, 8:32:19 PM4/6/16
to silverst...@googlegroups.com
I see. I definitely agree on consistency. With how long 4 is taking, on one hand it makes more sense to roll something like this into v4 because it's a common sense and modern practice. On the flip side, it may slightly delay the release of 4. However, I think it's worthwhile, especially if namespaces will be implemented by v4. Once you have namespaces, you can broaden your spectrum of exceptions. Until then, you can use your own custom extensions of Exception as well as PHP's built-in extensions to Exception; I see no issue with this. I have no idea why you cannot utilize the power of exceptions once you convert over.

I like the idea of this making its way into a utility function, but in all honestly, it's a utility function that really shouldn't exist.

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to silverstripe-d...@googlegroups.com.
To post to this group, send email to silverst...@googlegroups.com.
Visit this group at https://groups.google.com/group/silverstripe-dev.
For more options, visit https://groups.google.com/d/optout.

Reply all
Reply to author
Forward
0 new messages