When should assertions stay in production code? [closed]
Asked Answered
P

17

196

There's a discussion going on over at comp.lang.c++.moderated about whether or not assertions, which in C++ only exist in debug builds by default, should be kept in production code or not.

Obviously, each project is unique, so my question here is not so much whether assertions should be kept, but in which cases this is recommendable/not a good idea.

By assertion, I mean:

  • A run-time check that tests a condition which, when false, reveals a bug in the software.
  • A mechanism by which the program is halted (maybe after really minimal clean-up work).

I'm not necessarily talking about C or C++.

My own opinion is that if you're the programmer, but don't own the data (which is the case with most commercial desktop applications), you should keep them on, because a failing asssertion shows a bug, and you should not go on with a bug, with the risk of corrupting the user's data. This forces you to test strongly before you ship, and makes bugs more visible, thus easier to spot and fix.

What's your opinion/experience?

See related question here


Responses and Updates

An assertion is error, pure and simple and therefore should be handled like one. Since an error should be handled in release mode then you don't really need assertions.

That's why I prefer the word "bug" when talking about assertions. It makes things much clearer. To me, the word "error" is too vague. A missing file is an error, not a bug, and the program should deal with it. Trying to dereference a null pointer is a bug, and the program should acknowledge that something smells like bad cheese.

Hence, you should test the pointer with an assertion, but the presence of the file with normal error-handling code.


Slight off-topic, but an important point in the discussion.

As a heads-up, if your assertions break into the debugger when they fail, why not. But there are plenty of reasons a file could not exist that are completely outside of the control of your code: read/write rights, disk full, USB device unplugged, etc. Since you don't have control over it, I feel assertions are not the right way to deal with that.


Yes, I have Code Complete, and must say I strongly disagree with that particular advice.

Say your custom memory allocator screws up, and zeroes a chunk of memory that is still used by some other object. I happens to zero a pointer that this object dereferences regularly, and one of the invariants is that this pointer is never null, and you have a couple of assertions to make sure it stays that way. What do you do if the pointer suddenly is null. You just if() around it, hoping that it works?

Remember, we're talking about product code here, so there's no breaking into the debugger and inspecting the local state. This is a real bug on the user's machine.

Purpura answered 20/8, 2008 at 11:1 Comment(1)
There's an interesting related post at the Software Engineering SE (though the discussion is c++ focused): Should there be assertions in release buildsIncendiary
E
111

Assertions are comments that do not become outdated. They document which theoretical states are intended, and which states should not occur. If code is changed so states allowed change, the developer is soon informed and needs to update the assertion.

Elo answered 20/8, 2008 at 11:1 Comment(3)
@jkschneider: unit tests are for testing things within the scope of the program's code. assertions are for ensuring that assumptions which the code is based on are actually true in reality, before the code continues processing under those assumptions. They're "documentation" in the sense that, if they turn out not to be the case, the program will abort, state the assumption, and indicate that the assumption didn't hold up. You can also read the assertion as documention of that in the code, of course.Dolph
This is the best answer because it relates asserts to comments, which is a helpful way of thinking about them. They are a step up from comments because they are constantly machine tested during development, but they should always be meaningful to human readers first. Just like comments, they should not be part of logic or final execution. Just like comments, you can leave them in or take them out depending on whether the language is compiled or interpreted, your roll-out plans, obfuscation strategy, etc. I have seen one case where a comment actually caused a bug, but that was a weird one.Halimeda
More specifically, asserts are informational and not functional. An assert by itself has no effect on program flow or results. An exception on the other hand, alters program flow and thus results.Perugia
H
68

Allow me to quote Steve McConnell's Code Complete. The section on Assertions is 8.2.

Normally, you don't want users to see assertion messages in production code; assertions are primarily for use during development and maintenance. Assertions are normally compiled into the code at development time and compiled out of the code for production.

However, later in the same section, this advice is given:

For highly robust code, assert and then handle the error anyway.

I think that as long as performance is not an issue, leave the assertion in, but rather than display a message, have it write to a log file. I think that advice is also in Code Complete, but I'm not finding it right now.

Hymnody answered 20/8, 2008 at 11:1 Comment(1)
I think what the second quote from Code Complete means is that you should have the assert -- which will get compiled out in production code -- and you should also have "if (!condition) { AttemptGracefulRecovery(); }", i.e. don't let violation of program invariants crash the program.Perugia
T
43

Leave assertions turned on in production code, unless you have measured that the program runs significantly faster with them turned off.

if it's not worth measuring to prove it's more efficient, then it's not worth sacrificing clarity for a performance gamble." - Steve McConnell 1993

http://c2.com/cgi/wiki?ShipWithAssertionsOn

Thornton answered 20/8, 2008 at 11:1 Comment(5)
Actually, the worst thing that happens is when code crashes due to something that is NOT an assert. If the code will definitely crash later on with 100% probability, then the assert must absolutely be there. If you deref a pointer, then you have to implicitly assert that it isn't null before. If you divide by a number, you assert that it's not zero. Take out the asserts and all crash locations are UNDOCUMENTED. The real problem is not structuring the program to let subsystems crash and be restarted by a watchdog.Hollister
assert ref != null; is different than if (ref == null) throw new IllegalArgumentException(); You shouldn't be using the first for preconditions that could be false. You need to use assert for things that can't be false. Example, int i = -1 * someNumber; i = i * i; then later to remind people that i is positive, assert i > 0;Mas
"Actually, the worst thing that happens is when code crashes due to something that is NOT an assert. If the code will definitely crash later on with 100% probability, then the assert must absolutely be there." - this is a false dichotomy.Nonbeliever
@RobertGrant: For many programs, crashing is far from the worst thing that can happen. For a program that's supposed to check the soundness of a building or bridge design, wrongly reporting that a design is sound can be just about the worst thing it could do. For a program that's exposed to the outside world but has read-only access to confidential data, leaking that data may be worse than just about anything else the program could do. The notion that a crash with no meaningful indication of the cause is "the worst thing that could happen" ignores many dangers that are far worse.Allbee
@Allbee I wasn't agreeing with the comment I was quoting.Nonbeliever
I
23

If you're even thinking of leaving assertions on in production, you're probably thinking about them wrong. The whole point of assertions is that you can turn them off in production, because they are not a part of your solution. They are a development tool, used to verify that your assumptions are correct. But the time you go into production, you should already have confidence in your assumptions.

That said, there is one case where I will turn assertions on in production: If we encounter a reproducible bug in production that we're having a hard time reproducing in a test environment, it may be helpful to reproduce the bug with assertions turned on in production, to see if they provide useful information.

A more interesting question is this: In your testing phase, when do you turn assertions off?

Ileneileo answered 20/8, 2008 at 11:1 Comment(7)
I think that assertions should never be included in production code. assertions are NOT errors, they are designed for developers. Assertions should only live in test code. Having an app crash is because of and assertion fail unacceptable and sloppy development. Developers need to go the extra mile to handle errors gracefully.Weatherley
If it is inevitable that you will crash given a null pointer passed into a fn; there isn't a choice about dealing with it explicitly. You either have some way of gracefully handling the condition (because it can come from input from the outside world), or you crash at a DOCUMENTED location with an assert, rather than at a random spot that may have corrupted things along the way. how assert is handled should be a per-module decision though. Maybe your watchdog restarts process, or you wipe a chunk of memory for that module to reset it to start state (soft object "reboot").Hollister
I think this is a limited view of the use of asserts. I always log the asserts both to console and cloud storage, and leave them on in production. Leaving asserts on confirms that my assumptions remain correct even in production code and in production usage. Just because code ran a few times successfully in debug with asserts on doesn't mean users won't find a way to pass different values through the same code path.Filter
The whole point of the assert statement is that you can turn the checks on or off. If you leave them on in production, why use the assert statement?Ileneileo
The only extent to which this answer actually evaluates any real good or bad effects of leaving them on in production, is in pointing out that one scenario where turning them on in production does some good. Therefore without intending to, this answer only servers to promote leaving them on all the time, since it does not cite any harmful effects of doing so.Denton
Assertions often slow the system down. Since they're not designed for production, they are free to be slow and inefficient, which may be necessary to perform certain tests. For example, Microsoft once added a fast-recalculation feature to Excel. When one cell changed, this feature limited recalculation to just the cells that needed it. They tested this with an assertion that recalculated the entire spread sheet and compared the results. This made the development version run really slowly, but it also weeded out a lot of bugs. When they released this feature, it proved to be very reliable.Ileneileo
While it's useful to have development-only assertions, it's also useful in production code to have "last ditch emergency backstop" checks before a program does something harmful. Such checks should never fail unless system state is compromised, but--especially for code which faces the outside world--may greatly reduce the danger from attackers who figure out how to compromise system integrity. If a language had an assertion construct that would allow assertions to fire early at an implementation's convenience, the runtime costs could be much lower than for "ordinary" checks.Allbee
C
16

Assertions should never stay in production code. If a particular assertion seems like it might be useful in production code, then it should not be an assertion; it should be a run time error check, i.e. something coded like this: if( condition != expected ) throw exception.

The term 'assertion' has come to mean "a development-time-only check which will not be performed on the field."

If you start thinking that assertions might make it to the field then you will inevitably also start making other dangerous thoughts, like wondering whether any given assertion is really worth making. There is no assertion which is not worth making. You should never be asking yourself "should I assert this or not?" You should only be asking yourself "Is there anything I forgot to assert?"

Continuo answered 20/8, 2008 at 11:1 Comment(0)
M
10

Unless profiling shows that the assertions are causing performance problems, I say they should stay in the production release as well.

However, I think this also requires that you handle assertion failures somewhat gracefully. For example, they should result in a general type of dialog with the option of (automatically) reporting the issue to the developers, and not just quit or crash the program. Also, you should be careful not to use assertions for conditions that you actually do allow, but possibly don't like or consider unwanted. Those conditions should be handled by other parts of the code.

Mavilia answered 20/8, 2008 at 11:1 Comment(2)
As I see it, the primary purpose of a production assertion is as an emergency backstop: allowing the program to continue is very likely to cause harm that is sufficiently serious that preventing it is more important than anything else the program might be doing. Having a good error message would be nice, if possible, but that's of only secondary importance.Allbee
The whole point of the assert keyword is so you can turn them off in production. If you want to leave them in production, don't use an assert statement.Ileneileo
S
7

In my C++ I define REQUIRE(x) which is like assert(x) except that it throws an exception if the assertion fails in a release build.

Since a failed assertion indicates a bug, it should be treated seriously even in a Release build. When my code's performance matters, I will often use REQUIRE() for higher-level code and assert() for lower-level code that must run fast. I also use REQUIRE instead of assert if the failure condition may be caused by data passed in from code written by a third party, or by file corruption (optimally I would design the code specifically to be well behaved in case of file corruption, but we don't always have time to do that.)

They say you shouldn't show those assert messages to end-users because they won't understand them. So? End users may send you an email with a screen shot or some text of the error message, which helps you debug. If the user simply says "it crashed", you have no ability to fix it. It would be better to send the assertion-failure messages to yourself automatically, but that only works if (1) the software runs on a server you control/monitor or (2) the user has internet access and you can get their permission to send a bug report.

Square answered 20/8, 2008 at 11:1 Comment(4)
They also say you shouldn't show those assert messages to hackers because they are valuable clues for breaking in.Halimeda
These days, when most applications are client server, you can add the assertion failure to a database, so you don't need to show them to the user to get feedback.Ileneileo
@Halimeda consider whether the alternative is better. If the assertion is omitted in the final build, the code runs with invalid state and the outcome is likely unpredictable - a potential vulnerability. And if the bug isn't reported, how will it get fixed? Typically, normal users greatly outnumber crackers so that the bug is more likely to be encountered by a normal user first. Except if the bug only happens when a cracker is trying to hack it, in which case, at least you detected a problem and aborted the operation.Square
If assertions exist in production code, they should instead add to a log which should be forwarded to the dev team for analysis. This should be done either automatically (which means the SW should have some sort of agreement to this on installation), semi-automatically (which requires some reporting tool) or manually (done when the user reports it to support). The user really shouldn't be seeing them as it serves to not tell them anything useful at best or confuse them at worst. In either case, not a good UX design.Whinny
E
4

If you want to keep them replace them with error handling. Nothing worse than a program just disappearing. I see nothing wrong with treating certain errors as serious bugs, but they should be directed to a section of your program that is equipped to deal with them by collecting data, logging it, and informing the user that your app has had some unwanted condition and is exiting.

Eddaeddana answered 20/8, 2008 at 11:1 Comment(0)
C
2

Suppose a piece of code is in production, and it hits an assertion that would normally be triggered. The assertion has found a bug! Except it hasn't, because the assertion is turned off.

So what happens now? Either the program will (1) crash in an uninformative way at a point further removed from the source of the problem, or (2) run merrily to completion, likely giving the wrong result.

Neither scenario is inviting. Leave assertions active even in production.

Chastise answered 20/8, 2008 at 11:1 Comment(0)
D
2

Our database server software contains both production and debug assertions. Debug assertions are just that -- they are removed in production code. Production assertions only happen if (a) some condition exists that should never exist and (b) it is not possible to reliably recover from this condition. A production assertion indicates that a bug in the software has been encountered or some kind of data corruption has occurred.

Since this is a database system and we are storing potentially enterprise-critical data, we do whatever we can to avoid corrupted data. If a condition exists that may cause us to store incorrect data, we immediately assert, rollback all transactions, and stop the server.

Having said that, we also try to avoid production assertions in performance-critical routines.

Deacon answered 20/8, 2008 at 11:1 Comment(2)
I would call your "production assertion" an "exception", and code it as such.Halimeda
You're probably right but the product was originally written in C. Even when we changed it to be C++ the compilers we used on some of our platforms didn't properly support exceptions. Most of the old code was not rewritten in C++ so these assertions are still used.Deacon
O
2

Provided they are handled just as any other error, I don't see a problem with it. Do bear in mind though that failed assertions in C, as with other languages, will just exit the program, and this isn't usually sufficient for production systems.

There are some exceptions - PHP, for instance, allows you to create a custom handler for assertion failures so that you can display custom errors, do detailed logging, etc. instead of just exiting.

Oesophagus answered 20/8, 2008 at 11:1 Comment(0)
C
1

I find it best to handle all errors that are in scope, and use assertions for assumptions that we're asserting ARE true.

i.e., if your program is opening/reading/closing a file, then not being able to open the file is in scope -- it's a real possibility, which would be negligent to ignore, in other words. So, that should have error-checking code associated with it.

However, let's say your fopen() is documented as always returning a valid, open file handle. You open the file, and pass it to your readfile() function.

That readfile function, in this context, and probably according to its design specification, can pretty much assume it's going to get a valid file ptr. So, it would be wasteful to add error-handling code for the negative case, in such a simple program. However, it should at least document the assumption, somehow -- ensure somehow --- that this is actually the case, before continuing its execution. It should not ACTUALLY assume that will always be valid, in case it's called incorrectly, or it's copy/pasted into some other program, for example.

So, readfile() { assert(fptr != NULL); .. } is appropriate in this case, whilst full-blown error handling is not (ignoring the fact that actually reading the file would require some error handling system anyway).

And yes, those assertions should stay in production code, unless its absolutely necessary to disable them. Even then, you should probably disable them only within performance-critical sections.

Cobden answered 20/8, 2008 at 11:1 Comment(0)
N
1

I see asserts as in-line unit tests. Useful for a quick test while developing, but ultimately those assertions should be refactored out to be tested externally in unit tests.

Niels answered 20/8, 2008 at 11:1 Comment(1)
Assert: State a fact or belief. They're not for testing (only), but for stating what you think is true (i.e., your assumptions), so that your program will not continue if your assumptions are wrong for some reason. For example, this is a valid use of assertions: assert(pow(1,0) < 1). It's not really an appropriate place for error checking, because if that's not true, then pretty much all of modern math is wrong, and... well, how would you even begin to handle that? Handling that incorrect assumption is outside the scope of the program; you take it on faith. But you verify, anyway.Dolph
C
0

ASSERTIONS are not errors and should not be handled as errors. When an assertion is thrown, this means that there is a bug in your code or alternatively in the code calling your code.

There are a few points to avoid enabling assertions in production code: 1. You don't want your end user to see a message like "ASSERTION failed MyPrivateClass.cpp line 147. The end user is NOT you QA engineer. 2. ASSERTION might influence performance

However, there is one strong reason to leave assertions: ASSERTION might influence performance and timing, and sadly this sometimes matter (especially in embedded systems).

I tend to vote for leaving the assertion on in production code but making sure that these assertions print outs are not exposed to end user.

~Yitzik

Correct answered 20/8, 2008 at 11:1 Comment(3)
No, assertions are for assumed facts. If our code returns 27 when it's assumed it will ALWAYS return 25, the problem could also be a bug in our physical assumptions about the universe: maybe those two bits switched to the 5 possible value, for the first time in the history of computing. The assertion is there to confirm that your code is still operating under the assumptions it was written for. If physics goes out the window, your code should notice, leave the drive alone and quit while it's ahead ;) But yes, it's not an error in the code, and what kind of error handling could you do?Dolph
Allow my to refine my opinion: 1. Assertion do check our assumptions. If our assumptions our wrong, this means that there is a bug in OUR code. 2. Our code should not assert usage of our code. i.e. a function should not fail on assertion if something is wrong in the input from user. We will return error and user should handle (he can assert success) 3. I prefer leaving assertion in production - but behavior will probably be modified. I agree that there is no appropriate error handling. Failed assertion == bug.. but the system might re-init itself instead of stopping and waiting for reboot.Correct
it doesn't NECESSARILY mean that there's a bug. For instance, many of the projects I'm involved in come with a list of assumed facts in the documentation. Those assumptions are there to protect developers' code from being called buggy, when business people might have told them the wrong thing, or when no information is available from third parties about particular variables, for instance. Assertions can be used to verify that the program should / should not run, that third-party systems are correct, not just whether IT is correct.Dolph
M
0

Most of the time, when i use assertion in java (the assert keyword) I automatically add some production codes after. According to the case, it can be a logging message, an exception... or nothing.

According to me, all your assertions are critical in dev release, not in production relase. Some of them must be kept, other must be discarded.

Madoc answered 20/8, 2008 at 11:1 Comment(0)
G
0

I rarely use assertions for anything other that compile time type checking. I would use an exception instead of an assertion just because most languages are built to handle them.

I offer an example

file = create-some-file();
_throwExceptionIf( file.exists() == false, "FILE DOES NOT EXIST");

against

file = create-some-file();
ASSERT(file.exists());

How would the application handle the assertion? I prefer the old try catch method of dealing with fatal errors.

Gem answered 20/8, 2008 at 11:1 Comment(1)
Exceptions are for unusual situations that you expect to encounter in a working application, like in your example here. Assertions are for situations that you expect to never encounter. So, if you do encounter them, there must be a bug in your code. In your example, and exception is clearly the right approach. But assertions are still very useful to catch bugs.Ileneileo
S
-9

An assertion is error, pure and simple and therefore should be handled like one.

Since an error should be handled in release mode then you don't really need assertions.

The main benefit I see for assertions is a conditional break - they are much easier to setup than drilling through VC's windows to setup something that takes 1 line of code.

Stgermain answered 20/8, 2008 at 11:1 Comment(2)
Using asserts as conditional breakpoints is really annoying for several reasons. Most important one is that these asserts confuse other developers in the team - how would they know if it's an error when that assert fired or is it that someone left his conditional breakpoint into the code?Shanklin
There wouldn't be if asserts weren't in the code in the first place. And if you were using them to monitor code only you would see them (unless you were checking them into the source tree). I've worked in places which has asserts for practically everything. Having to click about 60 times at the start of the program because the help documentation server is non-available gets tiresome very quickly.Stgermain

© 2022 - 2024 — McMap. All rights reserved.