What type of result should be returned from the service layer?

Say, for example, that I have a PostsService that creates a post:

public class PostsService : IPostsService
{
    public bool Create(Post post)
    {
        if(!this.Validate(post))
        {
            return false;
        }

        try
        {
            this.repository.Add(post);
            this.repository.Save();
        }
        catch(Exception e)
        {
            return false;
        }
    }
}

The problem with this is that if an exception is thrown during the repository actions, it's swallowed. Create() returns false and all that the consumer knows is that the Post wasn't added, but doesn't know why.

Instead, I was think of having a ServiceResult class:

public class ServiceResult
{
    public bool Success { get; private set; }
   开发者_JAVA技巧 public Exception Exception { get; private set; }
}

Would this be a good design? Or do I even need to report these errors to the consumer? Is it sufficient to say "An error occurred while adding the post." and then log the exception inside of the service?

Any other suggestions are appreciated.

I think it depends on the scope of why. Some failure reasons shouldn't concern the consumer, in my opinion and these should be logged in the service. On the other hand, there are cases when the consumer should be informed of these reasons, so in addition to logging them, you should indicate the cause of the error somehow.

For instance, suppose a very common use case of a user registration service. It's quite likely that you'd have a unique attribute that identifies the user (user handle, email, etc). This uniqueness must be enforced by the service (and probably at a database level too). Now, if the consumer is trying to register a user and it fails because the constrain was violated, the consumer should be notified of this reasons (so it could try some other user handle). Failed validations fall in the same scenario, most of the times.

If there's some kind of internal DB problem on the other hand, the consumer should not be informed of the details (because that's exclusive responsibility of the service; and it's not something the consumer can do anything about anyway).

Considering that, I'd say returning a boolean is insufficient in many cases. So having some kind of ServiceResult type doesn't sound like a bad idea. I'm not sure I would include the Exception though. But perhaps you can create some kind of ServiceExpection, specific to your service. Sometimes error codes are just fine for this, too.

It's a really bad idea to catch all exceptions. You only catch that which you can reasonably handle. You can't handle all exceptions, so why are you catching it? To prevent a stack trace?

If you don't want your users to see a horrible stack trace, I get that, but you do want it to die and die horribly so that something isn't corrupted.

Let's say that service threw an OutOfMemoryException, you just caught it, and won't know whether or not your application is leaking memory like a sieve or is improperly allocating large objects.

That's a bad thing.

If you don't want your user to see what was wrong, then you should change your catch to say:

catch (Exception e)
{
    //send it to yourself, log it. Just don't swallow it.
    LogErrorAndEmailDevTeam(e); 
    throw new SaveFailedException("We're sorry. \n" + 
    "It's not your fault, but something bad happened.\n\n" + 
    "We've been notified and will fix it as soon as possible.");
}

But even better would be to let the application die a horrible death instead of catching that exception so that you know quickly when something goes wrong.

You don't want your application to continue in a corrupted state, but that's what catch (Exception) does. Are you really sure you can handle whatever is thrown?

Yes, the ServiceResult class is a good idea, but I wouldn't include an exception, there are times you want to report errors but you don't want/need to create/throw/catch an exception, instead I would include a collection of error messages or an enum type indicating what went wrong, something like MembershipCreateStatus.

Also, don't catch Exception, it can make bugs hard to find.

Keep in mind that one of the key benefits of a service oriented architecture is decoupling components of the system. The more the calling layers have to know about the service the more tightly coupled the layers become. Following this logic it is better to have the calling layer know as little as possible about the error, all it needs to know is that the service call failed.

Does the calling layer really need to know why the service failed? What can it really do about it? Is it just so you can show a better message to the user? In this case does the user really care about the detail? It's easy to think the user wants to know more, just because you as the developer does. But developers should be getting information about error messages from logs, not from end user messages.

(Disclaimer: In my experience I've worked a lot more with exposing/consuming services between environments, where the built-in functionality in something like WCF isn't always useful and one doesn't always have control over the client consuming the service.)

More and more in my service designs I've moved towards a request/response system. (Actually, I've made heavy use of the agatha-rrsl library in this regard.) In that design, I have a BaseRequest object and a BaseResponse object, from which all request and response types inherit.

Continuing with what you were thinking, that ServiceResult class would serve well as the start of such a BaseResponse. It doesn't necessarily need the actual exception, but a few things I tend to include are:

1. A bool indicating success or failure for any request, even one which returns actual results on success.
2. An IList<> of custom Message objects, which themselves contain a message type of some sort (Info, Warn, Error, etc.) as well as the message text (and perhaps an additional user-friendly message text for UI display needs), stack trace if relevant, etc.

Going further, I also like to include a few things in the BaseRequest such as the originating user, host, etc.

The idea is that the service itself should never throw a raw exception. Whoever is consuming the service should always expect to get a valid response, even if that response indicates a failure of some sort. It should expect some basic information to come back every time and know how to handle that information.

If you're using WCF, I'd recommend against a ServiceResult, or anything similar for dealing with exceptions in a SOA layer.

You should define a FaultContract for your method. This would allow consuming code to know about the types of exceptions that can be thrown and handle them accordingly using traditional try/catch blocks.

This StackOverflow question has a full implementation of Fault Contracts.

You could still have your front-end simply say "An error has occurred", but using a fault contract would allow for better error handling on the UI layer of your code if it becomes needed in the future.