Exceptional Dependency Decency

Libraries can become less attractive to 3rd party integrators if they depend on too many unwanted other elements. This is especially true for libraries that are themselves pulled in as a dependency. Our horde/exception library is no exception to this. It is pulled in by almost all horde libraries because it is horde’s goto solution for exception hierarchies. A third party user of a library might not be particularly interested in horde’s custom exceptions. In Horde/Yaml: Graceful degradation I detailed why the yaml parser now works without its horde-native peers, including the horde/exception and horde/util libraries.

Dependency Hell: L’enfer, c’est les autres packages

Many Horde libraries started out as breakout parts from a monolithic framework. Drivers for various edge cases came to live in with their parents, interfaces, base classes, default implementations. Only rarely a very specialized driver was factored out into separate libraries, usually when it came as a late addition. As a result, the Horde 5 framework sometimes feels a bit like Linkedin: An item is likely related to most others by at most three or four intermediates. You invite one into your ecosystem and a sizeable portion of the whole clan arrives.

In the PEAR past, the cost of maintaining many micro-libraries and assembling a working installation from them was higher than today. Horde already contains about 140+ parts. Understanding the past is the first step.

Dependency Decency: Less is more

A necessary next step is to analyze the status quo and improve the future. Which dependencies are useful for the core business of a package? Which dependencies are only relevant to a specific sub class or driver? That member of the family maybe should have some private place to assemble its own friends, uncrowded by his relatives’ friends? A library is more likely to be adopted if you can consume the wanted aspects without having to live with anything unwanted.

Case Study: The horde/exception library

The horde/exception library is used in virtually all horde packages. It pulls in the horde/translation library, which is also used in most of the framework. This is no additional burden in the framework use case, but otherwise needs a second thought.

Should a library tightly couple its preferred translation mechanism?

Looking closer, there are only two translation keys, “Not Found” and “Permission Denied”. All other exceptions are translated at the level of the library or application that uses them or simply not at all. Maybe exceptions don’t want to be translated outside of the application calling context. In most places where an uncaught exception ought to be visible or logged, showing the English original is preferable or at least acceptable. Normal users should only ever see sanitized and scrubbed messages even in common error cases – and these should be translated and amended to be useful.

The NotFoundException is only translated if it is created without an input message, defaulting to “Not Found” (or translated equivalent). The same is true for PermissionDeniedException.
Besides that, I think we can provide more powerful and useful exception graphs by moving to interfaces and traits, away from inheritance.

“Not Found” by itself, in any language, is not very useful. What was not found? Where or why was it looked up? Who should be concerned? “Permission denied” – which permission is needed? What can I do about it?

Trying to be too nice sometimes results in being a burden

By translating early, we pull in a dependency when even the exception library itself is likely a pulled-in utility and not the first-class dependency conciously consumed. We also complicate translations by the mechanism of choice unless it happens to be horde itself. In many cases, this is not the best way to handle that. And even if we would provide substantial amount of translatable text with the library, we’d better offer the translations separately.

Promoting composition over inheritance: Do away with inheritable base classes.

The traditional model for horde library or app exceptions was to inherit from Horde_Exception or a few specialized other exceptions. There is an adapter for wrapping ancient PEAR_Error objects into proper exceptions and there is an adapter for consuming recoverable errors/warnings from fopen() and friends. Both become much less relevant in modern PHP. Apart from this, a library or app inherited from Horde_Exception or Horde_Exception_Wrapped and then may base its own specialized App_Exception_Foo on App_Exception, extends Horde_Exception. Unless, of course, a more appropriate builtin exception is thrown which will have no Horde specific interfaces at all. We can do better than that. We can provide the building blocks for libraries and applications to mark their relation to Horde or a specific sub system but still natively extend PHP’s most appropriate builtin.

Illustration

<?php

declare(strict_types=1);

namespace Horde\Exception;

// Import builtins to the namespace;
use Exception;
use LogicException;
use RuntimeException;
use Throwable;

/**
 * Base Throwable of all things Horde.
 * 
 * Quirk: Would even work without extending Throwable but that feels wrong.
 */
interface HordeThrowable extends Throwable 
{

}

/**
 * A library's or app's specific exception type
 */
interface DomainThrowable extends HordeThrowable 
{

}

/**
 * The most generic Horde\Exception
 */
class BaseException extends Exception implements HordeThrowable
{

}

/**
 * The most generic Horde\Exception for the "Domain" app or library
 */
class DomainException extends Exception implements DomainThrowable
{

}

/**
 * A Horde\Exception for the "Domain" app or library based on 
 * the builtin LogicException
 */
class DomainLogicException extends LogicException implements DomainThrowable
{

}

/**
 * Implements methods to digest an error_get_last() and either
 * throw an exception or return to normal code flow.
 */
trait WrapRecoverableErrorTrait
{
    ...
}

/**
 * The interface matching the implementing trait above
 */
interface WrapRecoverableErrorInterface
{
   ...
}

/**
 * Implements methods to wrap legacy a PEAR_Error class
 */
trait WrapPearErrorTrait
{
    ...
}

/**
 * A specific class for an app, library or subsystem with special 
 * methods to wrap a legacy warning or recoverable error. 
 * Could also extends any other PHP builtin class that matches more 
 * specifically the kind of error.
 */
class DomainIoErrorException extends RuntimeException
implements DomainThrowable, WrapRecoverableErrorInterface
{
    use WrapRecoverableErrorTrait;
}

try {
    /**
     * Some code does fopen() or similar which might result in a warning
     * The trait in DomainIoErrorException offers a static method that calls 
     * and evaluates error_get_last(). If it is OK, it just returns. 
     * Otherwise it throws an exception from the available data.
     */
    DomainIoErrorException::checkRecoverableErrors();
} catch (DomainIoErrorException $e) {
   // Offer a meaningful way to deal with failure to open the file 
   echo "caught IO error";
} catch (RuntimeError $e) {
   // This would have caught the DomainIoError, too.
} catch (HordeThrowable $e) {
   /* This catches almost anything horde-related. 
      You might want to log details and show a generic message to users */
    echo "caught generic horde error";
}

Refactor for elegance: Decomposing an inheritance graph into traits and interfaces.

The illustration above gives an idea how we can compose exceptions so that we have both at once: Use PHP builtin exceptions everybody knows and understands outside our ecosystem but also use interface hierarchies that help us to specifically handle exceptions from a subsystem. We also can amend any builtin exception with methods to add context, implicitly log issues or wrap legacy or foreign types of failure data. This allows for more elegant code that fits more nicely both into our own framework or third party use cases. This way the exception library only provides some very basic building blocks and leaves it to others to aggregate additional functionality as needed. It stays lean and free of scary dependencies. This way it can become a welcome guest in other ecosystems or at least one about which other integrators will not worry much. This goes both ways. A third party may contribute a single driver or plugin if there are good examples how to integrate them in the wider system.

As a side benefit, building a test environment becomes easier if your code does not need to be tested against too many different backends but everything is nicely separated.

But reality bites

In practice, we need to moderate our desire for change and improvement a little bit. While we should work towards eliminating the dependency on the Translation system, we should not break the existing behaviour without a transitional phase. There is a good and agreeable path towards it: We communicate our intention. We mark classes or calls that use translation as deprecated. We communicate what alternatives to use. We can even stub the translation system so it becomes optional – if it is not installed, the code simply won’t translate but otherwise behaves as expected.

We can provide the building blocks for a better exception system now and add our new base exception interface to the legacy classes. Consuming code can transition to checking against the interfaces without waiting for the throwing code to change all at once. In some cases that might mean that we need to delay introduction of return types. It is much less a problem with parameter signatures where inheriting classes always can accept a wider, less strictly defined set of inputs. By keeping the older interfaces around, we can make our more robust versions a matter of opt-in. We offer decent migration paths without withholding change altogether.

Comments

Leave a Reply

Your email address will not be published. Required fields are marked *