Fehlerbehandlung aus der Hölle

“Wer einen Fehler gemacht hat und ihn nicht korrigiert, begeht einen Zweiten.”

Konfuzius

Die Fehlerbehandlung in der Programmiersprache Java folgt einigen einfachen aber mächtigen Mustern. Dennoch zeigt sich immer wieder die Tendenz, auf erschreckende Weise davon abzuweichen.

Grundsätzlich kann man in der Fehlerbehandlung die zwei Situationen Fehlererkennung und Fehlerkorrektur unterscheiden. Wird im Programmablauf eine Konstellation erkannt, die einen Fehler darstellt, dann wird eine Exception geworfen.

public AncestorTree validate(AncestorTree tree) {
  if (cycleDetected(tree)) {
    throw new CyclicTreeException("cycle in tree " + tree.getId());
  }
  return tree;
}

In diesem Beispiel wird eine CyclicTreeException geworfen, weil im Stammbaum ein zyklische Verbindung entdeckt wurde. So etwas sollte nur bei der Existenz von Zeitreisenden auftreten. Die Verarbeitung wird an dieser Stelle abgebrochen und die Exception an die aufrufenden Methoden durchgereicht.

Bei der Fehlerkorrektur fängt eine aufrufende Methoden die Exception und führt Aktionen aus, um auf den Fehler zu reagieren.

public void handleNewTree(AncestorTree tree) {
  try {
    saveTree(validate(tree));
  } catch (CyclicTreeException e) {
    rejectTree(e);
  }
}

In diesem Beispiel wird ein valider Stammbaum gespeichert. Tritt aber bei der Validierung ein Fehler, in Form einer CyclicTreeException auf, dann wird der Stammbaum zurückgewiesen.

Obwohl oder gerade weil dieser Mechanismus für die Fehlerbehandlung sehr einfach erscheint, finden einige Code-Smells immer wieder ihren Weg in den Source Code.

Ein Teil dieser Code-Smells hat seinen Ursprung in der Unterscheidung von Checked und Unchecked Exceptions in Java. Unchecked Exceptions erben alle von der Klasse RuntimeException, daher werden sie häufig einfach Runtime Exceptions genannt. Sie können in jeder Methode geworfen werden. Checked Exceptions können aber nur in Methoden geworfen werden, die in ihrer Signatur darauf hinweisen.

public AncestorTree validate(AncestorTree tree) throws TreeException {
  if (cycleDetected(tree)) {
    throw new CyclicTreeException("cycle in tree " + tree.getId());
  }
  return tree;
}

In diesem Beispiel ist die CyclicTreeException eine Checked Exception, die von TreeException erbt. In dem throws Teil der Signatur könnte auch, an Stelle von TreeException, Exception oder CyclicTreeException stehen. Dies ist nicht nur ein klarer semantischer Unterschied, es hat auch einen starken Einfluss auf die aufrufenden Methoden.

Die TreeException in der Signatur signalisiert, dass diese Methode Unchecked Exceptions und alle Checked Exceptions werfen kann, die vom Typ TreeException sind. Beispielsweise sind das TreeException, CyclicTreeException und EmptyTreeException. Die aufrufende Methode muss diese Exceptions behandeln oder kann sie an eine weitere aufrufende Methode durchreichen. Durchgereichte Checked Exceptions müssen im throws Teil der aufrufenden Methode aufgeführt werden.

public void handleNewTree(AncestorTree tree) throws TreeException {
  try {
    saveTree(validate(tree));
  } catch (CyclicTreeException e) {
    rejectTree(e);
  }
}

In diesem Beispiel wird nur die CyclicTreeException behandelt und alle anderen TreeException durchgereicht.

Steht im throws Teil der Signatur Exception, dann kann diese Methode alle Checked Exceptions werfen. Sie zwingt damit alle aufrufenden Methoden dies auch zu tun oder alle Exceptions zu behandeln. Dies ist ein starker Code-Smell, weil er gravierende Auswirkungen auf die Code Pflege hat.

public void handleNewTree(AncestorTree tree) {
  try {
    saveTree(validate(tree));
  } catch (Exception e) {
    rejectTree(e);
  }
}

Die hier dargestellte Methode fängt Exception, weil die dazugehörige validate Methode Exception wirft. Während der Weiterentwicklung der Software passiert es nun, dass die saveTree Methode eine IOException werfen kann. Die neue Version wird produktiv geschaltet und plötzlich werden alle Stammbäume abgelehnt. Die Validierung scheint korrekt zu sein. Irgendwann erkennt jemand, dass hier eine IOException geworfen wird, weil die Festplatte voll ist. Bei einer validate Methode mit einer TreeException und einer saveTree Methode mit einer IOException, wäre der Entwickler gezwungen gewesen, die handleNewTree Methode um eine Behandlung einer IOException zu ergänzen.

public void handleNewTree(AncestorTree tree) throws IOException {
  try {
    saveTree(validate(tree));
  } catch (CyclicTreeException e) {
    rejectTree(e);
  }
}

Dies ist einer der Gründe, warum das Werfen von Exception aus Methoden tunlichst vermieden werden sollte. Ein anderer wichtiger Grund ist die Tendenz, dass nach geraumer Zeit, immer mehr Methoden Exception werfen. Dies ist dem Umstand geschuldet, dass Exceptions häufiger durchgereicht als behandelt werden.

Wie das Beispiel mit der Exception zeigt, sollte immer sehr spezifisch angegeben werden, welche Exceptions geworfen und gefangen werden. In dem Fall, dass die TreeException gefangen oder geworfen wird, ist der Entwickler auch nur so lange vor bösen Überraschungen sicher, bis es eine neue Unterklasse von TreeException gibt.

Der Vollständigkeit halber sollte hier aber noch eine Konstellation erwähnt werden bei der die Exception tatsächlich geworfen wird und wie man darauf verzichten kann.

public interface TreeSaver {
  AncestorTree save(AncestorTree tree) throws Exception
}

Das Interface TreeSaver wirft hier die Exception, weil davon ausgegangen wird, dass die implementierenden Klassen eine Checked Exception werfen könnten. Ein traditioneller Fehler ist hier, die Implementierungen auch Exception werfen zu lassen.

public class FileSaver extends AbstractFileSaver implements TreeSaver {
  public AncestorTree save(AncestorTree tree) throws Exception {
    return saveInternal(tree);
  }
}

Die Implementierungen von TreeSaver müssen jedoch nur die Exceptions angeben, die sie tatsächlich werfen.

public class FileSaver extends AbstractFileSaver implements TreeSaver {
  public AncestorTree save(AncestorTree tree) throws IOException {
    return saveInternal(tree);
  }
}

public class DatabaseSaver extends AbstractDatabaseSaver implements TreeSaver {
  public AncestorTree save(AncestorTree tree) throws SQLException{
    return saveInternal(tree);
  }
}

Leider bleibt damit das Problem erhalten, dass der Source Code, der mit dem TreeSaver agieren muss, auf Exception reagieren muss. Das Dilemma kann vermieden werden, wenn die Implementierungen auf Checked Exceptions verzichten und an dieser Stelle auf Unchecked Exceptions wechseln. So muss der DatabaseSaver die SQLExceptions selber verarbeiten und eine eigene Exception bereitstellen.

public class DatabaseSaver extends AbstractDatabaseSaver implements TreeSaver {
  public AncestorTree save(AncestorTree tree) {
    try {
      return saveInternal(tree);
    } catch (SQLException e) {
      throw new DatabaseException(e);
    }
  }
}

Der Ansatz erscheint sinnvoll, weil die tatsächlich geworfenen Checked Exceptions nicht domänenspezifisch für den TreeSaver sind, sondern einzig der jeweiligen Implementierung zur Laufzeit geschuldet sind.

Aus der Not haben viele APIs mittlerweile eine Tugend gemacht und verwenden nur Unchecked Exceptions.

Ein anderer Code-Smell der die Fehlerbehandlung regelmäßig heimsucht, ist die Organisation Exception und die Fehlercodes. Häufig ausgelöst durch Legacy Systeme werden Exceptions erstellt, die für diverse Fehler zuständig sind und diese durch einen Fehlercode repräsentieren. Dabei wird dann eine allgemeine Lösung für die gesamte Organisation präferiert und eine spezielle Exception etabliert.

public AncestryException extends Exception {
  public static final int UNKNOWN= 0;
  public static final int INVALID = 1;
  public static final int NO_OWNER= 2;
  public static final int CYCLE_DETECTED = 8;

  private final int errorCode;

  public AncestryException(int errorCode, String errorMessage) {
    super(errorMessage);
     this.errorCode = errorCode;
  }

  public int getErrorCode() {
    return errorCode;
  }
}

Die AncestryException sorgt für viele Probleme in der Entwicklung. Sie sorgt zuerst einmal für eine reduzierte Variante der Exception Problematik. Viele Methoden werfen und fangen die AncestryException und verwenden den Fehlercode bei der Fehlerkorrektur.

public void handleNewTree(AncestorTree tree) throws AncestryException {
  try {
    saveTree(validate(tree));
  } catch (AncestryException e) {
    if (e.getErrorCode() != AncestryException.CYCLE_DETECTED) { 
      throw e;
    }
    rejectTree(e);
  }
}

Da hier int Werte als Fehlercode verwendet werden, kann ein nachlässiger Entwickler statt der Konstanten einfach den numerischen Wert verwenden, statt AncestryException.CYCLE_DETECTED also einfach die 8. Oder ein Kollege verwendet eigene Werte, die nicht mit der Liste der Konstanten übereinstimmen. Auch könnte die Sammlung der numerischen Werte durch einen Kopierfehler Doubletten beinhalten.

Dann wird der numerische Wert durch einen Enum ersetzt, der aber die Situation nur bedingt verbessert. Entweder zwingt er die Entwickler, bei jeder neuen Fehlersituation eine neue Enum-Konstante zu etablieren oder, wie häufig beobachtet, es werden nur einige wenige Enum-Konstanten überhaupt verwendet. Beliebt sind dann INTERNAL_ERROR, UNKNOWN_ERROR und BUSINESS_ERROR.

Kompliziertere Varianten dieser Lösung versuchen mit Unterklassen und speziellen Nummernkreisen Überschneidungen und falsche Benutzung zu unterbinden, können aber die grundlegenden Mängel nicht beseitigen.

Ist die Organisation involviert, dann werden manchmal die Exceptions unnötig durch andere Exceptions ersetzt. Dem liegt die Idee zugrunde, dass jedes Modul seine eigenen Domänen Exceptions haben soll. Wenn dann Exceptions aus aufgerufenen Modulen gefangen werden, werden eigene weitergeworfen.

public void handleData(Data data) throws ModulAusgangException {
  try {
    eingang.handleData(data);
  } catch (ModulEingangIoException e) {
    throw new ModulAusgangException(ModulAusgangException.IO, "io fehler am eingang"); 
  } catch (ModulEingangParseException e) {
    throw new ModulAusgangException(ModulAusgangException.PARSE, e); 
  }
}

Im obigen Beispiel werden die ModulEingangIoException und die ModulEingangParseException gefangen und stattdessen eine ModulAusgangException geworfen. Das Problem mit dieser Lösung ist, dass in beiden Fällen die Verarbeitung des tatsächlichen Fehlers erschwert wird. Im ersten Fall besonders schwer, da die ursprüngliche Exception und ihre Informationen vollständig verworfen wird. Hier steht nur noch der Typ und eine generische Meldung zur Verfügung. Im zweiten Fall werden die Informationen zwar gerettet, aber die Verarbeitung auf höherer Ebene wird erschwert.

public void handleData(Data data)  {
  try {
    ausgang.handleData(data);
  } catch (ModulAusgangException e) {
    Throwable t = e.getCause();
    if (t instanceof ModulEingangParseException) {
      fixParseing(data, ((ModulEingangParseException)t).getErrorCode());
    } else if (e.getErrorCode() == ModulAusgangException.PARSE) {
      cleanInput(data, e.getMessage());
    }
  } 
}

Die Fehlerbehandlung ist relativ kompliziert geworden, weil nun interpretiert werden muss, was der eigentliche Fehler war. Entweder steht die Information in der ursprünglichen Exception oder in den Daten der gefangenen Exception. Was auch augenfällig ist, die Fehlerbehandlung muss nun nicht nur den Ursprungsfehler kennen, sondern auch die Marotten des zweiten Moduls beachten.

Manchmal wird der Code zur Fehlerbehandlung so kompliziert, dass der ErrorHandler zum Leben erweckt wird. Dann werden alle diese komplizierten Auswertungen in ein Drittsystem ausgelagert. Wie Frankensteins Monster agiert er grobschlächtig ohne genau Kenntnisse der Zusammenhänge. Wenn Systeme wachsen und neue Funktionalitäten und Fehlermöglichkeiten hinzukommen, wird häufig vergessen diesen Mechanismus zu ergänzen. Dann ärgern sich viele Anwender über die unzureichenden Meldungen zu unerwarteten Fehlern.

Zum Ende des Beitrags noch einige sehr kleine aber dafür umso ärgerlichere Code-Smells bei der Fehlerbehandlung.

Für manche einfach erscheinende Fehlerbehandlung wird die Exception gefangen und keine Fehlerbehandlung ausgeführt.

try {
  handle(data, parse(data.getValue()));
} catch (ParseException e) {
}

Zum Zeitpunkt der Entwicklung gab es die Anforderung, dass handle bei fehlerhaften Daten in data.getValue() nicht aufgerufen werden sollte. Das geschah durch das Werfen der ParseException in parse. Da nicht einmal der Fehler in das Log geschrieben wird, fällt dann später nicht auf, wenn eine ParseException in der handle Methode den Abbruch wegen einer fehlerhaften Implementierung verursacht.

Auch wenn ein Log Eintrag geschrieben wird, sollte hier weiterhin Misstrauen bestehen. Bei fehlerhaften Daten wird keine echte Fehlerbehandlung vorgenommen und so können in einem späteren Verarbeitungsschritt weitere Fehler auftreten.

Manche Entwickler sind sehr vorsichtig und überall, wo sie einen Fehler vermuten, wird eine Exception gefangen und im Log notiert. Da aber keine Fehlerkorrektur vorgenommen wird, wirft der Entwickler die Exception einfach weiter. Dann passiert es immer wieder, dass eine Exception mehrfach gefangen, ins Log geschrieben und weiter geworfen wird. Das Log wird geschwemmt mit Einträgen zu einem einzigen Fehler. Daher ist die Faustregel, es wird dort der Fehler ins Log geschrieben, wo er auch behandelt wird. Nicht dort wo die Exception erstellt wurde und auch nicht überall dort, wo sie nur durchgereicht wurde.

Wer sich bei dem einem oder anderen Code-Smell ertappt fühlt, dem sei das folgende Zitat ans Herz gelegt.

“Die schlimmsten Fehler werden gemacht in der Absicht, einen begangenen Fehler wieder gut zu machen.”

Jean Paul