Don’t Explode Your Java With Flawed Java Exception Code

Don’t Make These Mistakes In Production Java: Exceptions

Understanding how to use Java Exceptions well is critical to building maintainable, high-quality production software. This chapter of the Don’t Make These Mistakes In Production Java series captures the author’s hard-learned lessons building Java software in large-scale production environments. You can find this post and others in the series at http://mattmccann.io.

Read on to learn to more and improve your craft!

The Don’t Make These Mistakes In Production Java series assumes the reader has at least a beginner’s understanding of the Java programming language and does not belabor the language basics.

Avoid ScheduledExecutor Thread Death

The ScheduledExecutorService.scheduleAtFixedRate is a useful method for producing scheduled behaviors within a Java application, but it can bite unwary developers who do not carefully read the fine manual. Let's look at the key line of documentation:

…and so on. If any execution of the task encounters an exception, subsequent executions are suppressed. Otherwise…

Any Runnable implementation you submit to ScheduledExecutorService needs to have a Handler of Last Resort that handles any exception, notifies your on-call team engineer, and then most likely suppresses the exception.

What Not To Do

public class CheckTheLightsRunnable implements Runnable {
    ...
    @Override
    public void run() {
        // If this ever throws an exception, no more checking of the lights!
        lights.check(); 
    }
}

CheckTheLightsRunnable checkTheLights = new CheckTheLightsRunnable(lights);

ScheduledExecutorService n = Executors.newScheduledThreadPool(1);
scheduler.scheduleAtFixedRate(checkTheLights, 10, 10, SECONDS);

Do This Instead

public class CheckTheLightsRunnable implements Runnable {
    ...
    @Override
    public void run() {
        try {
            lights.check();
        } catch (InterruptedException e) {
            // Re-throw interrupts as this is a legitimate reason to cease execution
            throw e; 
        } catch (Exception e) {
            // It's not often that catching a generic Exception is reasonable, but
            // to avoid unexpected scheduled thread death, it's exactly what's called for.
            logging.error("Failed to check the lights", e);
            metrics.addCount(UNANDLED_EXCEPTION, true);

            // Note that the exception is not re-thrown!
        }
    }
}

CheckTheLightsRunnable checkTheLights = new CheckTheLightsRunnable(lights);

ScheduledExecutorService n = Executors.newScheduledThreadPool(1);
scheduler.scheduleAtFixedRate(checkTheLights, 10, 10, SECONDS);

Don’t Use Exceptions For Control Flow

This might seem obvious, but you’d be surprised how often this shows up in production code bases. Don’t use exceptions for control flow, they are very computationally expensive! There is almost always an exception-free approach to handling the control flow, usually by checking preconditions. Consider this real-world example of production code converting strings to LocalDateTime.

The code below accepted input from the caller as a string, and the string could be in one of two date formats. The developer’s mistake is that they try to convert the string using the first date format, and if that throws an exception, they try to convert using the second format. This method of flow control is incredibly expensive to execute, especially if the second date format is common as it means throwing and handling an exception with each execution.

What Not To Do

private static DateTimeFormatter TIME_FIRST_FORMAT = 
    DateTimeFormatter.ofPattern("HH:mm dd-MM-YYYY");

// asString can either by YYYY-mm-dd or HH:mm dd-mm-YYYY
public LocalDateTime convertToLocalDateTime(String asString) {
    try {
        // Note that this code is first trying to parse as local iso date
        return LocalDateTime.parse(asString, DateTimeFormatter.ISO_LOCAL_DATE);
    } catch (DateTimeParseException e) {
        // and the string isn't in local iso date, try the other format. This results
        // in non-exceptional code (the second date format) requiring an exception throw
        // as part of normal flow control. This is very expensive!
        return LocalDateTime.parse(asString, TIME_FIRST_FORMAT);
    }
}

Do This Instead

private static String ISO_LOCAL_PATTERN = "HH:mm dd-MM-YYYY"
private static DateTimeFormatter TIME_FIRST_FORMAT = 
    DateTimeFormatter.ofPattern(TIME_FIRST_PATTERN);

// asString can either by YYYY-mm-dd or HH:mm dd-mm-YYYY
public LocalDateTime convertToLocalDateTime(String asString) {
    // There are many ways we might make this function more elegant, but let's consider
    // just this simple alternative. Instead of try...catching, the code checks the length
    // of the string to determine which format to use.
    //
    // This code is functionally the same as the previous example, but is drastically
    // more efficient if the TIME_FIRST pattern is a common input!
    if (asString.length() == ISO_LOCAL_PATTERN.length()) {
        return LocalDateTime.parse(asString, DateTimeFormatter.ISO_LOCAL_DATE);
    } else {
        return LocalDateTime.parse(asString, TIME_FIRST_FORMAT);
    }
}

Don’t Create Zombie Threads; Honor InterruptedException

InterruptedException is thrown across a wide swath of the Java SDK. The exception is thrown when a Thread's work needs to be interrupted, with the most common reason being the shutdown of your software application. If you suppress or otherwise mistreat InterruptedExceptions, you'll find your code misbehaves on shutdown.

What Not To Do

Let’s consider the earlier example of the CheckTheLightsRunnable. Instead of using a ScheduledExecutor, imagine we implemented the Runnable as a polling worker.

public class CheckTheLightsRunnable implements Runnable {
    ...
    @Override
    public void run() {
        // Looping on while (true) is generally a bad practice, but let's accept it
        // in the spirit of illustrating the InterruptedException mishandling
        while (true) {
            try {
                lights.check();
                 
                // Again, polling with Thread.sleep is generally a bad practice, but 
                // this kind of code shows up in production all the time. Let's power
                // through these side yucks to focus on our zombie code.
                Thread.sleep(1000);
            // Notice that this catch block is catching Exception and suppressing it,
            // which would include InterruptedException
            } catch (Exception e) {
                logging.error("Failed to check the lights", e);
                metrics.addCount(UNANDLED_EXCEPTION, true);
            }
        }
    }
}

// Now our CheckTheLightsRunnable is doing it's work, checking those lights endlessly
ExecutorService executorService = Executors.newFixedThreadPool(1);
executorService.submit(new CheckTheLightsRunnable());
// On shutdown, we want to clean up and shutdown our thread pool
getRuntime().addShutdownHook(() -> {
    executorService.shutdownNow();
});

Do you see the flaw in the Exception handling block? ThreadPoolExecutor will call Thread.interrupt on the CheckTheLightsRunnable when the shutdown hook is called, causing an InterruptedException to be thrown inside the execution of run(). Unfortunately, the catch (Exception e) handler block will catch and suppress the interrupt, leaving the thread still executing. Depending on the runtime executing your Java application, this could mean your code will either never shutdown or will shutdown in messy manner, resulting in an unknown system state.

Don’t Use Exceptions To Return Missing Values

This mistake is minor, but is a pet peeve of the author’s. It may be tempting to throw an exception when a return value is missing, but this is rarely an appropriate pattern (unless the missing value is truly exceptional). Instead, prefer to return an Optional type which makes for much cleaner client code.

What Not To Do

// Service code
/**
 * Exception thrown when a request Book is missing.
 */
public class MissingBookException extends IllegalArgumentException {
    public MissingBookException(String asin) {
        super(String.format("Missing book with asin '%s'", asin));
    }
}

class BookClient {
    ...

    /**
     * This method tries to read a Book record from a DynamoDB table.
     *
     * @param asin The unique id of the Book
     * @returns The book record matching the provided asin
     * @throws MissingBookException Thrown when no book record is found to match the provided asin
     */
    public Book getBook(String asin) throws MissingBookException {
        Item bookItem = this.bookTable.getItem("asin", asin);
        if (bookItem == null) {
            throw new MissingBookException(asin);
        } else {
            return Book.build(bookItem);
        }
    }
}

// Client code
try {
    Book book = bookClient.getBook("myAsin");
    ...
} catch (MissingBookException e) {
    ...
}

There are several issues with this example:

• Consider how much boilerplate code had to be written just to accommodate that an ASIN may not have a corresponding book. The getBook function can't simply throw an IllegalArgumentException as the standard Java Exception class (or a sub-classing Exception) could be thrown by lower level code, causing unexpected handling behavior. This then requires a custom exception to be defined.
• The client is now forced to write two separate blocks of code, one to handle the expected case and another the exceptional. This structure is awkward and increases overall code complexity
• Throwing exceptions is expensive!

Do This Instead

// Service code
class BookClient {
    ...

    /**
     * This method reads a Book record from a DynamoDB table.
     *
     * @param asin The unique id of the Book
     * @returns The book record matching the provided asin
     */
    public Book getBook(String asin) {
        Item bookItem = this.bookTable.getItem("asin", asin);
    
        return Optional.ofNullable(bookItem).map(Book::build);
    }

// Client code
Optional<Book> book = bookClient.getBook("myAsin");
if (book.isPresent()) {
    ...
}

The Optional approach is simultaneously more succinct as well as self-documenting. It's clear that Book may or may not be provided, and Optional provides methods for handling the missing Book case.

Under-specifying Swallowed Exceptions

Swallowing exceptions is sometimes appropriate behavior, depending on the logic within your service, but it’s important to be as specific as possible about the Exception you are catching, lest you catch an exception you didn’t intend and obfuscate bugs.

What Not To Do

/**
 * This method will try to parse the configuration from the specified file.
 */
public Optional<Config> parseConfig(String configFileAsJson) {
    // Nevermind the side yuck of missing defensive programming

    ObjectMapper objectMapper = new ObjectMapper();

    try {
        File configFile = new File(configFileAsJson);
        Config config = objectMapper.readValue(configFile, Config.class);

        return Optional.of(config);
    // The intent of this catch is to handle cases in which the file
    // is inaccessible or contains malformed JSON, but it could catch
    // something else as well
    } catch (Exception e) {
        log.warn("Failed to parse the configuration from file");
        return Optional.empty();
    }
}

This catch block is catching and suppressing Exception and as a result, it will report any error as a failure to parse the configuration, which can be misleading. If configFileAsJson is null, a NullPointerException would be thrown but suppressed and treated as an unreadable config file, hiding a bug!

Do This Instead

public Optional<Config> parseConfig(String configFileAsJson) {
    // This is what should be happening to avoid those NPEs, but
    // let's imagine this was missed, as sometimes happens in production 
    // code!
    // checkNotNull(configFileAsJson);

    ObjectMapper objectMapper = new ObjectMapper();

    try {
        File configFile = new File(configFileAsJson);
        Config config = objectMapper.readValue(configFile, Config.class);

        return Optional.of(config);
    // This could be even more narrowly specified if we want separate
    // parsing behavior for missing files, inaccessible files, malformed
    // json etc
    } catch (IOException e) {
        log.warn("Failed to parse the configuration from file");
        return Optional.empty();
    }
}

In this modified code, the caught Exception is scoped down to IOException , allowing the NullPointerException to throw out of the method and avoiding the production of confusing log lines that might obfuscate a program bug. As a rule, unless you are writing a Handler of Last Resort, you should not be catching Exception, Throwable, RuntimeException .

In Conclusion

This post captures just a few mistakes the author has seen in real-world large scale production Java code and how that code handles exceptions. The reality is that these mistakes really do end up in production and contribute to increased code complexity, system instability, and other misbehaviors. Be a better software engineer and avoid these mistakes.

I’ll be writing more focused posts on Exception handling performance issues and other production Java topics. If these topics interest you, follow R. Matt McCann to be notified of new posts.