Error handling - help please?

[swaldman]

Does anybody have time to do a quick rundown on how error handling should be done in DW, going forward?

There are a number of different ways of doing things within the codebase at present. I've tended to adopt LJ::throw for new stuff because it seemed comprehensible(!), but Mark has recently said in a review "I... don't think we use LJ::Error anywhere useful anymore. It's pretty old.". (given where the comment was made I assume that it referred to a LJ::throw statement, which I imagine must be part of a LJ::Error package)

So I'm wondering what best practice is now, and thus what we should do in new code?

Thanks

Comments

[mark]

I was actually contemplating this after your reply to my code review comment.

Of late, I've just been using Carp's croak/confess or die (as appropriate). I think that exceptions (using Try::Tiny or similar) is just... a lot of effort for relatively little gain, when you can do similar with eval if you really want.

I would suggest that you do one of two things:

1) If you want to die from the perspective of the caller, use Carp's "croak". If you want to die with a backtrace, use "confess".

2) If you want to die from YOUR perspective, use "die".

Why would you pick one or the other? Think about it like this... the error message will include your message plus a file name and line number. What would be more useful -- seeing "MyModule.pm line 85" or seeing "whoever called me line 395"?

If it's the former, die. If the latter, use Carp.

Generally, the latter is probably correct -- since you want to say that the call to your method failed, and here's the reason why. Instead of saying that something in your method failed.

All of this is much easier than LJ::throw and dealing with error objects, which are a fancy way of propagating information -- but all we really need in 99% of situations is what was the failure, what failed, and where.

Does that all make sense?

Elaborating on what Mark said

[pauamma]

Here's an example of whether/when to use Carp::croak vs. die.

1- Your caller wants you to believe 6 impossible things before breakfast, and you can only do 5. (And your caller should know that, and should not ask you to do impossible things, only believe them.) Or one of those things that should be impossible is actually true. Clearly, it's your caller's fault, not yours. Use Carp::croak.

2- Your caller only handed you 5 impossible things to believe, but it turns out that you can't check whether those things are false, as they should be. (Perhaps you need a database or memcache to do that and it's broken.) That's beyond your caller's control, so you should just use die.

[swaldman]

It does - thank you :-)

[swaldman]

To build upon something that Fu queried in the same review: Does the same apply to errors in workers?

[mark]

Not really -- workers usually have their own sort of $job->failed("message") kind of semantics. In those cases, the error message also usually gets shown to the user (in permanent failures anyway), but even temporary failures show up in the error logs which are read by non-developers.

In workers, you should think about what kind of error it is -- temporary or permanent -- and make sure you always exit out through one of those methods, with an error message that is, hopefully, useful for the user.