The Downfall of Exception

Some time ago I came across a weird situation. I had a piece of PHP code which behaved really strange. Since the actual code was a rather complicated beast, I’ll use a simplified example. Let’s have three functions:

function a($file) {
    $result = fopen($file, "r");
    if ($result === false) {
        throw new \RuntimeException("The function failed");
    }
}
function b($file, Logger $logger) {
    $retries = 0;
    while (true) {
        try {
            a($file);
            break;
        } catch (\RuntimeException $e) {
            $retries++;
            $logger->info("Retrying");
            if ($retries > 10) {
                throw new MyOwnException("A failed");
            }
        }
    };
}
function c() {
    try {
        b("somefile", new Logger("app-name"));
    } catch (\Exception $e) {
        throw new UserException("b failed");
    }
}

So — I have a function a, which somewhere in its code uses a PHP function throwing a warning or notice — e.g. fopen. I check for the return value and if that function fails, I raise an exception. The outer function b catches the exception and retries the call if needed. So far, so good, ain't it? I also have a PHPUnit test for it:

public function testb() {
    $handler = new \Monolog\Handler\TestHandler();
    $logger = new \Monolog\Logger("test", [$handler]);
    try {
        b("non-existent-file", $logger);
        self::fail("Must throw an exception");
    } catch (MyOwnException $e) {
        self::assertTrue($handler->hasInfo("Retrying"));
    }
}

One day we discovered that the retry mechanism does not work. The code worked under normal conditions, but when an error occurred, a was not run again and MyOwnException was never thrown even though the fopen call was returning false.

Of course, there is a perfectly reasonable explanation. The code runs as part of a Symfony application which uses the set_error_handler function to set its own error handler to convert PHP notices, warnings and errors into ErrorException. That means that when fopen fails, and emits a warning, ErrorException is thrown at that line. The statement if ($result === false) is not executed. The catch (\RuntimeException $e statement does not catch it, so the execution goes directly into the c function and throw new UserException("b failed”). No retries. No mercy.

The solution is easy — use the silencer operator: $result = @fopen($file);, which tells the Symfony error handler not to throw the ErrorException.

The real mystery is: how come this passed the tests?

The answer is again in the error handler. When running PHPUnit tests, PHPUnit uses its own error handler to catch PHP notices, warnings and errors. This is the baby. It throws a Warning exception when a warning occurs, the Warning exception inherits from the Error exception and the Error exception inherits from the Exception. Aaaand … fanfare …
the Exception inherits from the RuntimeException class.

Therefore, in the test, the fopen line throws the Warning exception, which inherits from the RuntimeException, which is caught in the b function and the a function is retried. This also means that the line if ($result === false) is never executed in that test (though it is executed in other tests, so test coverage does not really help here).

Of course, all the problems were caused by my utter laziness and a number of bad things I did:

• When checking the return value, I should have put in the silence operator automatically, because the error condition is already checked.
• I was too lazy to create my own exception, so I threw a built-in RuntimeException exception. This was a really bad idea.
• Subsequently catching the general RuntimeException instead of a more specific one caused most of the trouble.
• Also catching Exception in c was a really bad idea too. I should have created my own hierarchy of exceptions for my library and catch only them.
• To top it off, I initially missed $parent in the thrown UserException, which made the mystery a lot more difficult to solve in the beginning.

The fixed code should look something like this:

class MyAFunctionException exends MyLibraryException {}
class MyBFunctionException exends MyLibraryException {}
function a($file) {
    $result = @fopen($file, "r");
    if ($result === false) {
        throw new MyAFunctionException("The function failed");
    }
}

function b($file, Logger $logger) {
    $retries = 0;
    while (true) {
        try {
            a($file);
            break;
        } catch (MyAFunctionException $e) {
            $retries++;
            $logger->info("Retrying");
            if ($retries > 10) {
                throw new MyBFunctionException("A failed", 0, $e);
            }
        }
    };
}

function c() {
    try {
        b("somefile", new Logger("app-name"));
    } catch (MyLibraryException $e) {
        throw new UserException("b failed", 0, $e);
    }
}