Subscribe to PHP Freaks RSS

or die() must die

Print
by Daniel on May 28, 2009 8:44:03 AM - 68,597 views

This is something I constantly see on the PHP Freaks forums, and I absolutely hate it. It's the infamous or die() statement.

$result = mysql_query('SELECT foo FROM bar', $db) or die('Query failed: ' . mysql_error($db));

I see it all the time, and I see people telling other people to do that all the time. It's plain simply bad practice and it's time that people start to understand this. When I confront people with it they usually say something along the lines of "oh, but it's just for debugging purposes". Okay, so I tend to put echo and var_dump() statements in my code for debugging as well. However, this is not the same. It's quite obvious that you wouldn't want an array var_dump'ed to the screen in the final application, but you certainly do want some sort of error handling for when e.g. database queries fail.

The or die() trick is a very poor choice for several reasons:

  1. It's not a very nice way to present the user with an error message.
  2. Using for instance the mysql_error() call with it, as many people do, exposes information that should never get output in a production environment (see: PHP Security)
  3. You cannot catch the error in any way.
  4. You cannot log the error.
  5. You cannot control whether it should be output to the screen or not. It's okay to do that in a development environment, but certainly not in a production environment.
  6. It prevents you from doing any sort of cleanup. It just ends the script abruptly.

So if your little or die() trick is just for debugging purposes, how are you going to handle potential errors after you've deployed your application? Fortunately, PHP does actually include a function that allows you to raise PHP errors on runtime: trigger_error()

This function allows you to raise errors of type E_USER_NOTICE, E_USER_WARNING and E_USER_ERROR. These behave exactly like their non-USER counterparts, i.e. errors of type E_USER_ERROR are fatal and halts execution while the other two doesn't. If you are used to doing the or die() it's also very easy to implement:

$result = mysql_query('SELECT foo FROM bar', $db) or trigger_error('Query failed: ' . mysql_error($db), E_USER_ERROR);

Syntactically this is very much like the previous code snippet, but much better. Because errors of these types behave like errors PHP would normally make you can also use all the facilities PHP has for error handling. You can implement a custom error handler so you can display nice messages to your user, and you can log the errors to a file. Finally you can disable output of errors in a production environment.

Another option is to use exceptions. Example:

if (!$result = mysql_query('SELECT foo FROM bar', $db)) {
	throw new Exception('You fail: ' . mysql_error($db));
}

Personally, I prefer exceptions because they're easily catchable using try-catch blocks, which makes it pretty easy to degrade if something unexpected, or rather an exception, something that doesn't behave as expected, occurs. As an added bonus you you will get a stack trace which aids in debugging.

A great example of exception usage could be database transactions using PDO (MySQLi supports it as well if you prefer that). When you normally run a query on a database server it will be committed instantaneously. When you start a transaction changes won't be committed until you explicitly say so. This allows you to rollback if you wish.

We might have a setup like this:

+---------+------------+-----------+-------+
| user_id | first_name | last_name | funds |
+---------+------------+-----------+-------+
|       1 | Daniel     | Egeberg   |   500 |
|       2 | John       | Doe       |   350 |
+---------+------------+-----------+-------+

We want to transfer 120 from Daniel to John. This involves two operations: 1) Removing funds from Daniel, and 2) Adding funds to John. We need both or none of those operations to finish successfully. Executing just one of them is not an acceptable scenario.

Here is our transfer script:

$db = new PDO('mysql:host=localhost;dbname=test', 'user', 'password');

$transferAmount = 120;
$fromUserId = 1;
$toUserId = 2;

try {
	$db->beginTransaction();
	
	$stmt = $db->prepare('UPDATE users SET funds = funds + :amount WHERE user_id = :user_id');
	
	// remove funds from sender
	$stmt->execute(array('amount' => $transferAmount * -1, 'user_id' => $fromUserId));
	
	// add funds to recipient
	$stmt->execute(array('amount' => $transferAmount, 'user_id' => $toUserId));
	
	$db->commit(); // we will only get to this if both the above queries executed successfully
}
catch (PDOException $e) {
	$db->rollback(); // damn... well, at least we can roll back
	
	echo 'Sorry, but we were unable to transfer the funds. Please contact customer support if the problem persists.';
	// log this incident somehow. further info is available using $e->getMessage()
}

Finally just a quick check that we did in fact transfer the money:

+---------+------------+-----------+-------+
| user_id | first_name | last_name | funds |
+---------+------------+-----------+-------+
|       1 | Daniel     | Egeberg   |   380 |
|       2 | John       | Doe       |   470 |
+---------+------------+-----------+-------+

Yup. Everything's good!

In this case our good friend or die() is simply not an option. If the first statement ran fine, but the second failed I would have just lost money on nothing, and because the programmer was too lazy to implement real error handling nobody would be able to figure out what happened, and the case couldn't be documented. I certainly would not appreciate that...

People need to stop that or die() nonsense, and even more importantly, people need to stop teaching other people their own bad practice. Calling die() when something happens is not an acceptable way of handling the situation.

Comments

Jeff Combs May 28, 2009 9:25:45 AM

Nice writeup Daniel. I think you gave a perfect use case and it makes perfect sense. I think you should extend this to a tutorial and write up a nice error handling class ;)

Gareth Evans May 28, 2009 9:37:39 AM

That was very nicely done. I usually check my result and if it failed run a method from my error object that stores the error to file, and implements a mailer object to let me know. This is a far cleaner and more comprehensive start to error handling and I will be implementing it in my next framework 'upgrade'.

On a similar note of 'not-to-do', I do wish people would stop executing the follwoing code;

$password = md5(mysql_real_escape_string($_GET['password']));

mds is only going to return a hash of the entered string, there is no want or need to escape the passed password string.

MichaƂ Jarosz May 28, 2009 11:13:08 AM

Oh yes. Let's make it exit.
When posting on phpFreaks forums I adapted to what seemed to be a de facto standard here, but I never really liked it. Glad to hear I was not the only one.

Chris Young May 28, 2009 10:56:10 PM

Good explanation with contextual reference.

gerrywastaken May 30, 2009 5:28:04 PM

I completely agree that die must die. On exceptions, your example is very good, but most usage of exceptions that I see are also quite bad as they just produce spaghetti code.

Something that I think is even worse than die is the use of the Error Control Operator (@). C&P of what I wrote in the PHP manual:

"Error suppression should be avoided if possible as it doesn't just suppress the error that you are trying to stop, but will also suppress errors that you didn't predict would ever occur. This will make debugging a nightmare.

It is far better to test for the condition that you know will cause an error before preceding to run the code. This way only the error that you know about will be suppressed and not all future errors associated with that piece of code.

There may be a good reason for using outright error suppression in favor of the method I have suggested, however in the many years I've spent programming web apps I've yet to come across a situation where it was a good solution..."

Thanks for doing your part to improve code quality in the php world, I only wish the developers of PHP would just throw out all the crap and force people to improve their code.

mgkimsal May 31, 2009 8:14:16 PM

In 2009, PHP books are still being published with this bad approach in them. This is one of the things that's got to stop ASAP to help prevent yet another generation of PHP developers from learning this. I realize not everyone learns directly from books, but it reinforces all the bad code out there, and continues to do so. IIRC, even one of the PHP6 books I saw simply showed "mysql_connect(...) or die()" as recommended db work. No PDO, no abstraction, no error handling. Pretty sad...

Chris Henry Jun 5, 2009 12:54:06 AM

die() has become a huge nuisance in a lot of production code that I manage. It's been used as the de facto error handler in place of logging, user feedback, etc. There's nothing worse than having a webpage die(), except for a webpage that dies with a smarmy 'Please contact support' message on a white screen.

markux^Hs Jun 18, 2009 6:52:59 AM

Yawn. Quit preaching. For simple debugging there's absolutely _nothing_ wrong with it.

Daniel Jun 18, 2009 8:26:24 AM

Then perhaps you can answer the question "how are you going to handle potential errors after you've deployed your application?"

You will go back and replace all of it? That's both error prone and tedious. Why not just write proper code from the start? Maybe you'll just leave it with no error handling at all? That's if possible even worse.

Adam Jun 24, 2009 9:41:25 AM

Whilst I do agree that die() has an awful look about it and should never be used in 'live' code, sometimes during debugging it is handy to be able to very simply kill the script when you get to a certain point in the code and be able to check the value of a var, print out an array, etc. Just to simply and quickly check the data is what you expect it to be, and also prevent any further processing that could produce files, insert into a database, etc. If you see my point?

Thanks for the blog though...

EDIT: Aaah silly me, just realised "or die".. Nevermind!

skatterbrainz Jun 26, 2009 7:50:23 PM

Wow. I have to admit I am guilty of this simply from following blogs, articles and other forums. This *really* opened my eyes. Great article - thanks!

Russell Crevatas Sep 21, 2009 12:25:10 AM

# You cannot catch the error in any way.
# You cannot log the error.

or die(theFunction(mysql_error()));

inside theFunction you could do both, but the rest of the points are rather valid :)

MatthewJ Oct 9, 2009 2:16:00 PM

Much more an issue of preference... It's kind of funny you think you can assume people will forget. The only reason I and most people I'm sure make the suggestion to people is for immediately debugging a problem. It is not my job to make sure you write good code, or yours to make sure I do. I guess I just assumed everybody who finds themselves to be good programmer's, just add proper handling before production. Like others, I agree that there are "better" ways, but if people need their hands held that badly they need to read a book ;). It's the same thing as "sanitize your input"... we all know we need to do it, but most of the time when just scripting something together for concept, I leave it out. In 9 years of writing code, I have yet to forget to put it in to a production script. Have more faith in people, they can surprise you ;)

Daniel Oct 9, 2009 3:32:33 PM

The people I'm talking about are not the people who have been programming for 9 years. You're missing the point...

Also, as you'll see in my post, I do not think it's a matter of preference, and I argued why it isn't. What is for instance the point of writing something, and then go hunting for it to replace it with something else all the time? Why not just write it properly the first time?

outis Nov 26, 2009 5:46:10 PM

The default error mode for a PDO object is PDO::ERRMODE_SILENT, which prints a warning but doesn't stop normal execution; in particular, it won't cause exceptions to be thrown. The PDO example needs a call to setAttribute:

$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

PDO::__construct can throw a PDOException, so the instantiation of a PDO object in the example should be moved to within the "try" block. Daniel, can you edit the post, as many people probably won't read the comments for corrections?

As for the detractors, this is an argument about best development practices, rather than about programming. There are plenty of approaches you can take that are valid when programming (that is, they produce a program that does what you want) that you shouldn't do in development. Bad practices require more work than other designs, produce brittle programs, increase maintenance and aren't future-proof. In this case, "die" is more work overall and can be brittle: the exception design only requires one try/catch for every chunk of calls to PDO methods; "die" (and "trigger_error", in fact) must be repeated with every error check.

Cardale Jan 10, 2010 12:28:22 PM

You can also use trigger_error as well which some may want to use rather than the Java style coding. http://php.net/manual/en/function.trigger-error.php

s0c0 Nov 11, 2010 5:50:26 PM

I agree. Whoever came up with the or die() should have their keyboard and mouse revoked from them for life.

iansane Sep 22, 2011 12:42:01 PM

mathewJ,

um... I am reading a book. It's 2011 now and this book says use or die(). It also does not say to use "or die()" for debugging only. It specifically says "this is how you connect to a database" and proceeds to show using the "or die()" method.

So I appreciate when someone points out the pros and cons as this article does.

Thank you

Add Comment

Login or register to post a comment.