Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

On a SO question I asked here about some code I was unsure about, someone replied "BTW, horrible code there: it uses the error suppressing symbol (@) a lot."

Is there a reason why this is bad practice? With things like:

$db=@new mysqli($db_info) or die('Database error');

, it allows me to display just a custom error message. Without error suppressing, then it would still display the typical PHP message of:

Warning: mysqli::mysqli(): php_network_getaddresses: getaddrinfo failed: No such host is known. in some\file\path on line 6

as well as 'Database error'.

Is error suppressing always bad, and if so, what specifically about the above is bad?

Update: the actual code that I'm using is:

or error('Datatabase error', 'An error occurred with the database' . (($debug_mode) ? '<br />MySQL reported: <b>' . $db->error . '</b><br />Error occurred on line <b>' . __LINE__ . '</b> of <b>' . __FILE__ . '</b>' : ''))

which removes all previous output and displays an error message. So the fact that the error message doesn't include details about what specifically happened (which people seem to be suggesting as a reason why error suppressing is bad) is irrelevant.

share|improve this question
7  
Is wearing a blindfold while you're driving bad practice? –  Damien Roche Nov 28 '13 at 23:01
    
How could this be a serious question? I guess if you like to write code that takes hours to debug, suppression of errors would be a good thing. Think about it, your app fails, corrupts data... and you don't want to know why or where in the code. When would that ever be a good idea? You must invite your friends over to perform a blanket party on you... it follows the same logic. –  Some Free Mason Nov 29 '13 at 14:13
    
@SomeFreeMason If I need to debug then I would just set $debug_mode to TRUE, since the actual code I'm using is: or error('Datatabase error', 'An error occurred with the database' . (($debug_mode) ? '<br />MySQL reported: <b>' . $db->error . '</b>' : '')) –  user107146 Nov 29 '13 at 16:17
    
@Paradoxical: I realise the state of exception/error handling in PHP is a bit FUBAR, but you are aware that you can turn visual display of errors off in your PHP configuration, right? –  Phoshi Nov 29 '13 at 16:45
    
@Phoshi unfortunately the hosting service I'm using doesn't give me access to php.ini –  user107146 Nov 29 '13 at 16:52

4 Answers 4

I think you're doing the right thing by suppressing the error, because you are implementing your own error handling.

It might be easier to consider the equivalent in, say, Java. Suppressing the error using @ is analogous to swallowing an exception. The following code, which is similar to yours, is reasonable:

try {
    db = new MySQLi(dbInfo);
} catch (SQLException connectionFailure) {
    die("Database error");
}

(Well, in Java, you wouldn't want the servlet engine to die, but you get the idea.)

However, this isn't acceptable:

try {
    db = new MySQLi(dbInfo);
} catch (Exception e) {
}

Most PHP error suppression is done carelessly, analogous to the latter example, hence the knee-jerk reaction against your code.

share|improve this answer
2  
I wouldn't call catching the exception and just stopping execution to be "handling" it. If you can solve the problem, great, do so. If you can't, what the hell are you doing catching it? This is what we have top level exception handlers for. Catching exceptions in this case is just making sure you have error handling code strewn all over your codebase. –  Phoshi Nov 29 '13 at 13:21

Error suppression is bad, because it does not only hide the information you already know (something went wrong). It may also hide information vital for debugging which you aren't aware of (what, where and why).

In this specific example, “Database error” isn't a very good error message. Something went wrong with the DB. Not very enlightening. The “Warning: mysqli::mysqli(): php_network_getaddresses: getaddrinfo failed: No such host is known. in some\file\path on line 6” is actually a better error message. It gives you a location and reason for the problem. You can jump right in and start debugging, because the error has already been located.

Of course, library designers sometimes have a tendency of bringing up to many irrelevant warnings. I don't know PHP sufficiently well to know if it has a way to filter warnings you are not interested in. I know that reading through a hundred-line stacktrace isn't very enlightening. But the correct response to too much information is not to reduce the amount of information to zero, but to filter out irrelevant parts at some stage. The @ operator does not have this granularity (it's motto is all or nothing), and is therefore not a suitable tool for effective error management.

There are some cases where you know that a certain error will turn up, and that fixing the actual problem is more expensive than silencing the message (and suffering potential fallout from that). This could be the case in a one-off script, but sticking your fingers into your ears and going “la la la” is not a professional response to (potential) bugs.

share|improve this answer
5  
You don't want to tell your users the exact error. You inform them the error is on your side and that you're working on it. You could log the errors on your server and set up alerts so you'll automatically know about it, but there's no reason to show these errors to the user. –  Jeroen Vannevel Nov 28 '13 at 20:18
1  
On a live site, it would surely be better for an error message that the typical user can understand to be displayed, rather than "some technical garbage about a mysqli, whatever that is", though? If I'm trying to debug it then I would remove the error suppression, but in a live site, I disagree that the full error should be displayed. –  user107146 Nov 28 '13 at 20:21
3  
@Paradoxical On a serious site, you wouldn't display an error message like "database error" and nothing else. You'd display a generic "internal server error" page with lots of extra fluff, styling, footer, etc. which die is not suitable for either. And the detailed information should go into a log regardless of what you show to the user. –  delnan Nov 28 '13 at 20:34
1  
Is there no way to present a user-friendly message on the screen and write the technical message to a log file? –  FrustratedWithFormsDesigner Nov 28 '13 at 20:35
3  
display_errors off, log_errors on, and you're pretty much good to go, do as you please on detecting an error, but don't throw them away. –  Wrikken Nov 28 '13 at 23:23

Supressing errors is bad because it hides the problem, which many times we are not even aware of and it may result in unexpected behavior that may prove very costly in some critical applications eg in applications that are used in financial, health and defence domains.

It is also not a good idea to have unhandled exceptions in the code and show the actual error messages to the user as it may result in security issues because error messages usually reveal a lot about your code, which may help malicious users and hackers to manipulate the system.

So the good practice is to handle the errors at different levels eg at highest level you may handle the error by just loggin the actual error and showing a simple and user friendly message to the user.

share|improve this answer

Yes, the error suppressing operator is generally a bad idea.

You should manage error reporting in the configuration (php.ini). So you can choose a different setting for each environment (for ex. leave warnings in development and hide them in production).

The only situation when using @ could make sense is when you develop a library for other developers. If you voluntarily choose to do something which produces a warning, and don't want to annoy the users of your library with a warning that does not depend on them, @ could be a solution.

I can't think of any other use.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.