Converting stacktrace to string

Question

I am just sharing the method that i am using to convert stacktrace to string. This stacktrace is then being used in AWS Lambda to log. There are two things i am mainly concerned about

private String convertStackTrace(Throwable throwable){
 StringWriter stringWriter = new StringWriter();
 PrintWriter printWriter = new PrintWriter(stringWriter);
 String stackTrace;
 try {
 throwable.printStackTrace(printWriter);
 stackTrace = stringWriter.toString();
 }
 catch(Exception e){
 logSelf("Error converting exception. Simple exception message will be logged");
 stackTrace = throwable.getMessage();
 } finally {
 try {
 stringWriter.flush();
 stringWriter.close();
 printWriter.flush();
 printWriter.close();
 } catch (Exception e){
 logSelf("Error closing writers");
 }
 }
 return stackTrace;
}

Here is logSelf method

private void logSelf(String error){
 lambdaLogger.log(formatMessage(
 this.getClass().getCanonicalName(), LOG_LEVEL_ERROR, error, null)
 );
}

1. Shall i be opening/closing those printwriters everytime an error is logged?
2. Is it correct way to convert stacktrace to string?

Answer 1

Yes, you should be opening/closing the printwriters each time.

Yes, conceptually it's a decent way to convert the stack trace..... but... there are concerns, and is there a better way?

The most significant concern is that, if there's a problem writing to the PrintWriter, the call throwable.printStackTrace(printWriter); will fail, and the method will return a default value. This is not ideal.

The reality, though, is that those methods can never fail because the IO is to a StringWriter, and there's no actual IO component. A failure there would be..... inconceivable. None of your exception handling can really happen... it's just not going to fail (the "Titanic Logger").

The issue is that your code is checking for impossible conditions in a way that's overkill.

Still, using some Java 7 semantics, you can use try-with-resource functionality, and get a much more concise method:

private static String convertStackTrace(Throwable throwable) {
 try (StringWriter sw = new StringWriter();
 PrintWriter pw = new PrintWriter(sw)) {
 throwable.printStackTrace(pw);
 return sw.toString();
 } catch (IOException ioe) {
 // can never really happen..... convert to unchecked exception
 throw new IllegalStateException(ioe);
 }
} 

Note that the method is now static, and it does not need the finally section to clean up.

You can see this running in ideone: https://ideone.com/rKj9mT

• Em Ae: Thanks. But you aren't closing PrintWriter and StringWriter. Does it matter here? I would go with approach of not using try/catch block
• rolfl: @EmAe - I am closing them.... that's what the try (... ) { ... } with resources does: docs.oracle.com/javase/tutorial/essential/exceptions/…
• Em Ae: Oh, I see. So what if i don't add try/catch block. Since they aren't adding any value. Shall i close them explicitly?
• Imus: Since the throwable.printStackTrace( ) throws an exception you have to catch it anyway. Since the introduction of the try with recourses in java 7, this construct is the preferred way. Although I would just leave the catch block empty except for the comment that it cannot happen in this case.