Errors, exceptions in services, best practices

Topics: Writing modules
Developer
Sep 7, 2011 at 10:50 AM

Hi all!

My question would be about the best practices of exception and error handling in services. I have seen various approaches when I dug through Orchard and many modules, so I'm a bit in doubt.

Exceptions in services

What to do if an exception (like some file IO or network problem) occurs in a service? I would say (because services are used by other program parts that interact with the user, but services are not the program parts that should directly interact with the user) if an exception occurs, an exception (maybe a new, wrapping the original one) should be thrown (an than caught by the code using the service).

So my questions would be:

  1. Is it good to throw exceptions from a service method? (I would definitely say yes.)
  2. If yes, what type of exception should they be? Of course they should be the most appropriate type, but should we use OrchardException or any built-in exception (like a general ApplicationException) is also good (including custom exceptions that derive from the former ones)?

Errors in services

I'm not sure the word "error" is appropriate, or "cause" would be better. This would be something that's not an exception (so it's part of the normal program flow), and a bit similar to validation. An example: user registration is rejected because a user with the same name exists (just made up). So there is some problem that the service method should report back to its caller. What would be the best approach? I have seen all of them in various modules.

  1. Using the notifier: I don't think this is the right approach as this is an interaction with the end user, what in my oppinion shouldn't be the task of a service. In addition, if the service method uses the notifier to report that something couldn't be completed then its caller code won't be aware of that.
  2. Returning a bool (indicating success or failure) and filling a property (maybe Error, or Cause) with the description.
  3. Returning a custom info object that encapsulates the status (success/failure) and on failure its cause. (I would use this one.)

The example of checking the existence of a user is actually solved in Orchard.Users with a separate service, so the whole notification is not an issue (as this is then task of the controller). But sometimes such "validation" only makes sense in the same service method what we are calling e.g. from a controller or from a driver.

What do you think?

Thanks in advance for everybody who took time to read through this long post :-).

Developer
Sep 8, 2011 at 2:56 PM

This page gives some hints in the case of "causes", but I'm not sure what would be to implement this. Of course I could copy the code from the example and use it happily, but isn't there any convention or ready solution in Orchard?

Coordinator
Sep 8, 2011 at 8:37 PM

That's the usual distinction between exceptions (which should be raised in exceptional circumstances), and errors, which are part of the normal operation of the application. The former should be thrown, logged and in some cases surfaced to the user. The latter should be surfaced to the user (typically through the notifier), but in a friendly way that preferably provides a workaround or suggestion on how to solve the problem.

Developer
Sep 8, 2011 at 11:15 PM

Thanks for the detailed reply!

Are there any advised practices regarding exceptions? I've seen that they are caught by the framework and in case of OrchardExceptions sometimes also presented to the end user via the notifier (?). Should module developers use OrchardException (or derivatives) or any appropriate built-in exception?

I understand that reporting an error to the end user is typically done by the notifier, but should this be the case even in a service (I'm speaking about the service itself, not anything using the service; that means I only don't like a service method directly using the notifier)? I have the above mentioned problems with this approach.

Coordinator
Sep 8, 2011 at 11:41 PM

Use OrchardException instead of Exception or ApplicationException if you want to report an explicit issue you have found. If you expect an exception, catch it. If you can recover, do it, if you can't, log it and rethrow it from your controller for instance, letting ASP.NET error pipeline redirect to a custom page.

If you look into the dev branch, you will find some refactoring regarding the exceptions.

Developer
Sep 8, 2011 at 11:53 PM

Thanks for the new details! And I will take a look at dev, sounds interesting.

Developer
Apr 18, 2012 at 7:58 PM
Edited Apr 18, 2012 at 8:00 PM

This is an old discussion of mine but I'm still a bit confused regarding exception usage best practices in Orchard.

Considering that

  1. Interfaces have nothing to bother about concrete implementations and
  2. One should only catch exceptions that one can handle (so rarely the base Exception)

...I see an issue about injected (using their interface) dependencies and exception handling. If I call a method on an injected object I have no clue what type of exception it can throw. The interface tells nothing about this, only if it contains comments about exceptions, which in turn exposes information about the underlying implementation (that could or could not be correct, depending on what an implementation is currently used).

Now what to do for consistent exception handling?

In my own code I could use the convention to only let OrchardException (and possible derivatives) escape the interface, possibly containing the lower-level exception in the InnerException if there was one. This could actually work well I think...

But what about using other APIs? For example I often use the ugly

            try
            {
                _storageProvider.DeleteFolder(path);
            }
            catch (Exception)
            {
            }

"pattern" to delete a folder that may be absent (IStorageProvider not having a FolderExists() what I've implemented in a fork in December BTW ;-)). This would also catch an OutOfMemoryException too and that would be rather bad... Now I could make the assumption that an IStorageProvider implementation throws an IOException derivative, but this depends purely on the implementation...

What do you think?

BTW there is a question on StackOverflow about pretty much exactly this.

Developer
May 1, 2012 at 2:07 PM

Anybody with some experience on this?

Developer
Jun 29, 2012 at 9:59 AM

Now this can be considered more or less solved, as the exceptions can be checked with the IsFatal() extension method.

Developer
Jul 22, 2012 at 6:45 PM

See a blog post about this here.