14

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.

6
  • 16
    Is wearing a blindfold while you're driving bad practice?
    – user9682
    Nov 28, 2013 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. Nov 29, 2013 at 14:13
  • 1
    @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, 2013 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, 2013 at 16:45
  • @Phoshi unfortunately the hosting service I'm using doesn't give me access to php.ini
    – user107146
    Nov 29, 2013 at 16:52

8 Answers 8

15

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.

3
  • 4
    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, 2013 at 13:21
  • Error != Exception Sep 6, 2020 at 18:21
  • maybe it is not right to implement "your own" error handling
    – user184868
    Jun 2, 2022 at 19:43
11

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.

8
  • 6
    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. Nov 28, 2013 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, 2013 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.
    – user7043
    Nov 28, 2013 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? Nov 28, 2013 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, 2013 at 23:23
1

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.

1

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.

0

I think we can formalize the reason that error suppression is bad because unknown unknowns happen.

Today you suppress the error. Then an unknown unknown happens, that line of code now causes a fatal error. And that puts you in the worst situation possible for system maintainers: e.g. the software stops working and nothing shows up in the logs.

Error suppression makes it impossible to maintain the good programming practice that "if things don't work, there will likely be something in the error logs".

Don't suppress errors, instead create code that doesn't cause any errors.

Additionally, make sure that any errors automatically get emailed to someone, so they can proactively fix them. Errors should be dealt with, not suppressed.

0

Suppressing errors (exceptions in your example cases, actually) AS WELL as warnings is a double edged sword. It helps your user-end application look clean, as you can basically assign custom error codes to a lot of pre-defined exceptions, but it can create issues when you, as a developer, are trying to debug an application.

Like, take a look at Microsoft Windows for example. When you encounter an exception, like a BSOD, you get an error code, as well as a certain string which lets you get a brief idea on what the exception is (for example, the one which I seem to encounter most often, DPC_WATCHDOG_VIOLATION). Now, I don't really have an operating system development experience, but it should be pretty clear that the actual exception that was encountered in this case would've had a much bigger error message, but it was suppressed and handled by passing through a predefined template, which gave us the error's Stop Code as well as a QR link to the error support page.

This is what I would say, good exception handling, since they know its an exception that can be encountered, have a template for it to send the user to the support page, and a user-readable error message that one can Google easily.

But what about an exception that does not have a predefined template? Would you throw a generic error message such as "operating system error"? Personally, that would be a design decision then, as to how you want to handle exceptions you don't have a predefined template for.

A good practice would be to still print the Exception occurred, but somewhere inside a log (systemd allows us to check logs with journalctl wherever systemd is used). For Java, there is Logger. For PHP, there is Monolog.

Also, what about warnings? Warnings can be suppressed in most cases, until they are critical. But its still a good practice to still log them using a logger, into a file.

0

Two different things: One, an API reports something that it calls an error. Two, an API reports an error.

An example was an API that I used to make sure that a file with some name didn’t exist. The name of the API contained “delete”. It reported an error if a file with the given name didn’t exist, or if the directory where the file would have been didn’t exist. At least the first case was no error to me since after the call, the file didn’t exist; exactly what I wanted. The second case, you have to decide yourself if it is an error to you or not. On the other hand, no permission to read the directory is an error.

So there are rare cases where you ignore some errors. But in general, you don’t. The very least is some logging in a development version, preferably falling into the debugger.

There are also APIs where some errors are “normal”. For example the iOS keychain API. Calls can fail because the user has no password set, has no finger print stored, has old hardware, and lots of things that are “normal”. As a developer you care just about failure or success, not the reasons. And then there are possible errors that you have never seen, and you can’t ignore them. So you write your own API that calls the original API, ignores the ignorable errors, and reports the ones you can’t ignore.

0

I have a huge amount of methods to which i pass an arbitrary amount of key/value pairs.

in cases where i don't discriminate between null,0,false,array() or basically anything empty() or !isset(), i use

    if (@ $params['key'])  

without error suppression, a warning is issued when the key does not exist. but i don't care whether it is unset or empty.

this here is bloaty. i don't like it at all:

    if (isset($params['key']) && $params['key'])  

this is even more bloaty:

    $p_a = $p_b = null;
    extract($attributes, EXTR_OVERWRITE | EXTR_PREFIX_ALL, 'p_');
    if ($p_a) { ; }

personally, i find the @ operator very appealing here. the only warning this would ever suppress is a warning i couldn't care less about.

but it might still be bad style, and i wonder whether there is a more elegant approach.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.