Why is exception.printStackTrace() considered bad practice?

Question

There is a lot of material out there which suggests that printing the stack trace of an exception is bad practice.

E.g. from the RegexpSingleline check in Checkstyle:

This check can be used [...] to find common bad practice such as calling ex.printStacktrace()

However, I'm struggling to find anywhere which gives a valid reason why since surely the stack trace is very useful in tracking down what caused the exception. Things that I am aware of:

1. A stack trace should never be visible to end users (for user experience and security purposes)

2. Generating a stack trace is a relatively expensive process (though unlikely to be an issue in most 'exceptional' circumstances)

3. Many logging frameworks will print the stack trace for you (ours does not and no, we can't change it easily)

4. Printing the stack trace does not constitute error handling. It should be combined with other information logging and exception handling.

What other reasons are there for avoiding printing a stack trace in your code?

Solution 1

Throwable.printStackTrace() writes the stack trace to System.err PrintStream. The System.err stream and the underlying standard "error" output stream of the JVM process can be redirected by

• invoking System.setErr() which changes the destination pointed to by System.err.
• or by redirecting the process' error output stream. The error output stream may be redirected to a file/device
  • whose contents may be ignored by personnel,
  • the file/device may not be capable of log rotation, inferring that a process restart is required to close the open file/device handle, before archiving the existing contents of the file/device.
  • or the file/device actually discards all data written to it, as is the case of /dev/null.

Inferring from the above, invoking Throwable.printStackTrace() constitutes valid (not good/great) exception handling behavior, only

• if you do not have System.err being reassigned throughout the duration of the application's lifetime,
• and if you do not require log rotation while the application is running,
• and if accepted/designed logging practice of the application is to write to System.err (and the JVM's standard error output stream).

In most cases, the above conditions are not satisfied. One may not be aware of other code running in the JVM, and one cannot predict the size of the log file or the runtime duration of the process, and a well designed logging practice would revolve around writing "machine-parseable" log files (a preferable but optional feature in a logger) in a known destination, to aid in support.

Finally, one ought to remember that the output of Throwable.printStackTrace() would definitely get interleaved with other content written to System.err (and possibly even System.out if both are redirected to the same file/device). This is an annoyance (for single-threaded apps) that one must deal with, for the data around exceptions is not easily parseable in such an event. Worse, it is highly likely that a multi-threaded application will produce very confusing logs as Throwable.printStackTrace() is not thread-safe.

There is no synchronization mechanism to synchronize the writing of the stack trace to System.err when multiple threads invoke Throwable.printStackTrace() at the same time. Resolving this actually requires your code to synchronize on the monitor associated with System.err (and also System.out, if the destination file/device is the same), and that is rather heavy price to pay for log file sanity. To take an example, the ConsoleHandler and StreamHandler classes are responsible for appending log records to console, in the logging facility provided by java.util.logging; the actual operation of publishing log records is synchronized - every thread that attempts to publish a log record must also acquire the lock on the monitor associated with the StreamHandler instance. If you wish to have the same guarantee of having non-interleaved log records using System.out/System.err, you must ensure the same - the messages are published to these streams in a serializable manner.

Considering all of the above, and the very restricted scenarios in which Throwable.printStackTrace() is actually useful, it often turns out that invoking it is a bad practice.

---

Extending the argument in the one of the previous paragraphs, it is also a poor choice to use Throwable.printStackTrace in conjunction with a logger that writes to the console. This is in part, due to the reason that the logger would synchronize on a different monitor, while your application would (possibly, if you don't want interleaved log records) synchronize on a different monitor. The argument also holds good when you use two different loggers that write to the same destination, in your application.

Solution 2

You are touching multiple issues here:

1) A stack trace should never be visibile to end users (for user experience and security purposes)

Yes, it should be accessible to diagnose problems of end-users, but end-user should not see them for two reasons:

• They are very obscure and unreadable, the application will look very user-unfriendly.
• Showing a stack trace to end-user might introduce a potential security risk. Correct me if I'm wrong, PHP actually prints function parameters in stack trace - brilliant, but very dangerous - if you would you get exception while connecting to the database, what are you likely to in the stacktrace?

2) Generating a stack trace is a relatively expensive process (though unlikely to be an issue in most 'exception'al circumstances)

Generating a stack trace happens when the exception is being created/thrown (that's why throwing an exception comes with a price), printing is not that expensive. In fact you can override Throwable#fillInStackTrace() in your custom exception effectively making throwing an exception almost as cheap as a simple GOTO statement.

3) Many logging frameworks will print the stack trace for you (ours does not and no, we can't change it easily)

Very good point. The main issue here is: if the framework logs the exception for you, do nothing (but make sure it does!) If you want to log the exception yourself, use logging framework like Logback or Log4J, to not put them on the raw console because it is very hard to control it.

With logging framework you can easily redirect stack traces to file, console or even send them to a specified e-mail address. With hardcoded printStackTrace() you have to live with the sysout.

4) Printing the stack trace does not constitute error handling. It should be combined with other information logging and exception handling.

Again: log SQLException correctly (with the full stack trace, using logging framework) and show nice: "Sorry, we are currently not able to process your request" message. Do you really think the user is interested in the reasons? Have you seen StackOverflow error screen? It's very humorous, but does not reveal any details. However it ensures the user that the problem will be investigated.

But he will call you immediately and you need to be able to diagnose the problem. So you need both: proper exception logging and user-friendly messages.

---

To wrap things up: always log exceptions (preferably using logging framework), but do not expose them to the end-user. Think carefully and about error-messages in your GUI, show stack traces only in development mode.

Solution 3

First thing printStackTrace() is not expensive as you state, because the stack trace is filled when the exception is created itself.

The idea is to pass anything that goes to logs through a logger framework, so that the logging can be controlled. Hence instead of using printStackTrace, just use something like Logger.log(msg, exception);

Solution 4

Printing the exception's stack trace in itself doesn't constitute bad practice, but only printing the stace trace when an exception occurs is probably the issue here -- often times, just printing a stack trace is not enough.

Also, there's a tendency to suspect that proper exception handling is not being performed if all that is being performed in a catch block is a e.printStackTrace. Improper handling could mean at best an problem is being ignored, and at worst a program that continues executing in an undefined or unexpected state.

Example

Let's consider the following example:

try {
  initializeState();

} catch (TheSkyIsFallingEndOfTheWorldException e) {
  e.printStackTrace();
}

continueProcessingAssumingThatTheStateIsCorrect();

Here, we want to do some initialization processing before we continue on to some processing that requires that the initialization had taken place.

In the above code, the exception should have been caught and properly handled to prevent the program from proceeding to the continueProcessingAssumingThatTheStateIsCorrect method which we could assume would cause problems.

In many instances, e.printStackTrace() is an indication that some exception is being swallowed and processing is allowed to proceed as if no problem every occurred.

Why has this become a problem?

Probably one of the biggest reason that poor exception handling has become more prevalent is due to how IDEs such as Eclipse will auto-generate code that will perform a e.printStackTrace for the exception handling:

try {
  Thread.sleep(1000);
} catch (InterruptedException e) {
  // TODO Auto-generated catch block
  e.printStackTrace();
}

(The above is an actual try-catch auto-generated by Eclipse to handle an InterruptedException thrown by Thread.sleep.)

For most applications, just printing the stack trace to standard error is probably not going to be sufficient. Improper exception handling could in many instances lead to an application running in a state that is unexpected and could be leading to unexpected and undefined behavior.

Solution 5

I think your list of reasons is a pretty comprehensive one.

One particularly bad example that I've encountered more than once goes like this:

    try {
      // do stuff
    } catch (Exception e) {
        e.printStackTrace(); // and swallow the exception
    }

The problem with the above code is that the handling consists entirely of the printStackTrace call: the exception isn't really handled properly nor is it allowed to escape.

On the other hand, as a rule I always log the stack trace whenever there's an unexpected exception in my code. Over the years this policy has saved me a lot of debugging time.

Finally, on a lighter note, God's Perfect Exception.

View more solutions

Comments

[Chris Knight]

Good point. Abusive use of printStackTrace() could blow our logging files, or at the very least fill it with reams of useless information

[Chris Knight]

Valid point, though we carefully watch our production console output.

[OCB]

Can you explain what you mean by carefully watch? How and who? At what frequency?

[Chris Knight]

Great answer, thanks. However, while I agree that most of the time its noise or not necessary, there are those times when its absolutely critical.

[Chris Knight]

By carefully watch, I should have said when requires. Its a rolling log file which is one of several ports of call if something goes wrong with our app.

[Vineet Reynolds]

@Chris Yes, there are times when you cannot but use System.out.println and Throwable.printStackTrace and of course, developer judgement is required. I was a bit worried that the part about thread-safety was missed. If you look at most logger implementations, you'll notice that they synchronize the part where log records are written (even to the console), although they do not acquire monitors on System.err or System.out.

[Chris Knight]

Excellent point. I'd hate to try to look at two (or more) interlace stack traces! What joy that would be...

[ESRogs]

Would I be wrong to note that none of these reasons apply to the printStackTrace overrides that take a PrintStream or PrintWriter object as a parameter?

[Geek]

@VineetReynolds I do have a follow up question. What did you mean by "System.err stream and the underlying standard "error" output stream? Aren't they the same?

[Vineet Reynolds]

@Geek, the latter is the process file descriptor with value 2. The former is just an abstraction.

[Geek]

@VineetReynolds Thanks for the follow up.+1.

[eis]

This. Quite often just not catching the exception at all, at least on method level, is better than catching, doing print of stack trace and continuing as if no problem occurred.

[Blasanka]

your answer is the one that I got.

[awwsmm]

"most 'exception'al circumstances". nice.

[EyouGo]

In JDK source code of printStackTrace(), it use synchronized for lock the System.err PrintStream, so it should be a thread-safe method.

[MasterJoe]

"exception is not allowed to escape" - did you mean that the exception is propagated up the stack ? how is logging different from printStackTrace() ?

[MasterJoe]

@ChrisKnight - could you please suggest an article which explains how logging files could get blown up with useless info ? thanks.

[Daniel Le]

JDK 7 was the latest version back in 2011, and Throwable.printStackTrace() synchronizes on the PrintStream's lock. System.err is one such PrintStream. Most of the content of the second and third edits is incorrect and should be rolled back. (The first version of the answer was actually helpful.)

[john ktejik]

dead link, the exception wasn't perfect

[Janusz Januszewski]

This makes no sense, because leaving only a message in logs doesn't exhaust the topic of error cause. Sometimes without displaying stacktrace fixing a bug is impossible.

[M.Ed]

@john ktejik try this one