Die Code-Überprüfungs-Checkliste, die ich nach 2.000 Pull-Requests erstellt habe

Ich habe 2.147 PR-Kommentare, die ich über einen Zeitraum von 18 Monaten hinterlassen habe, kategorisiert. 34% betrafen die Fehlerbehandlung. 22% betrafen die Benennung. Nur 8% beschäftigten sich mit der Leistung. Das hatte ich erwartet, als ich begann, meine Bewertungen zu verfolgen. Ich dachte, ich würde architektonische Probleme und komplexe Fehler finden. Stattdessen wies ich immer wieder auf dieselben grundlegenden Probleme hin: fehlende Nullprüfungen, vage Variablennamen und Fehlermeldungen, die den Benutzern nichts Nützliches sagten. Nachdem ich dieselben Probleme in Produktionsvorfällen gesehen hatte, wurde mir klar, dass dies keine kleinen Nichtigkeiten waren - sie waren der Unterschied zwischen Systemen, die sich anmutig verhalten, und solchen, die dich um 3 Uhr morgens aufwecken.

Warum die meisten Code-Überprüfungs-Checklisten am Ziel vorbeigehen

Alle sagen dir, du sollst auf den Code-Stil, die Testabdeckung und die Dokumentation achten. Das ist in Ordnung, aber dort bricht die Produktion nicht. Die Checklisten, die ich in Ingenieurbüchern sehe, konzentrieren sich auf das, was leicht messbar ist, anstatt auf das, was tatsächlich wichtig ist. Sie sagen dir, du sollst überprüfen, dass Funktionen weniger als 50 Zeilen haben, aber sie sagen dir nicht, dass du überprüfen sollst, ob die Fehlerbehandlung tatsächlich jemandem hilft, das Problem um 2 Uhr morgens zu debuggen, wenn Protokolle dein einziger Freund sind.

Die besten Code-Überprüfungen verhindern nicht nur Fehler – sie verhindern die Art von Fehlern, die zu Vorfällen führen. Eine fehlende Nullprüfung ist nicht nur ein potenzieller Absturz; es ist ein potenzielles Datenkorruptionsereignis, wenn dieser Absturz mitten in einer Transaktion auftritt.

Ich begann, meine PR-Kommentare zu verfolgen, weil ich frustriert war. Ich überprüfte Code, genehmigte ihn und sah dann, wie derselbe Entwickler den gleichen Fehler im nächsten PR machte. Das Feedback blieb nicht hängen. Also baute ich eine Tabelle. Jeder Kommentar, den ich hinterließ, wurde kategorisiert: Fehlerbehandlung, Benennung, Testen, Sicherheit, Leistung, Architektur oder anderes. Nach sechs Monaten hatte ich genug Daten, um Muster zu erkennen. Nach achtzehn Monaten hatten sich diese Muster zu einer Checkliste verdichtet, die tatsächlich funktionierte.

Der überraschende Teil war nicht nur, was an der Spitze der Liste steht - es war, was nicht dort steht. Kommentare zur Leistungsoptimierung machten nur 8% meiner Reviews aus. Sicherheitsprobleme waren 6%. Das sind die Dinge, über die wir uns in Architektur-Besprechungen den Kopf zerbrechen, aber in den täglichen PRs sind sie selten. Was häufig vorkommt? Entwickler, die den glücklichen Pfad annehmen, Dinge schlecht benennen und Fehlermeldungen für sich selbst schreiben, anstatt für die Person, die um Mitternacht debuggen wird.

Die Fehlerbehandlungs-Muster, die die Produktion immer wieder beeinträchtigen

Hier ist meine nummerierte Liste von Fehlerbehandlungsproblemen, die nach der Häufigkeit, mit der sie tatsächliche Vorfälle verursachen, eingestuft sind:

1. Stille Fehler in Hintergrundjobs. Der Code fängt eine Ausnahme, protokolliert sie und fährt fort. Scheint vernünftig, bis du merkst, dass dieser Job eine Zahlungsbestätigungs-E-Mail senden sollte. Jetzt denkt dein Kunde, dass er nicht belastet wurde, aber das wurde er. Ich sehe dieses Muster in 40% der Pull-Requests zu Hintergrundjobs. Die Lösung ist einfach: Wenn die Operation kritisch ist, fange die Ausnahme nicht ab - lasse sie hochsteigen und dein Alarmsystem auslösen. Wenn es nicht kritisch ist, dokumentiere, warum es in Ordnung ist, still zu scheitern.
2. Generische Fehlermeldungen, die den Kontext verbergen. "Es ist ein Fehler aufgetreten" sagt mir gar nichts. "Zahlungsabwicklung fehlgeschlagen" ist besser, aber immer noch nutzlos. "Konnte Karte mit Endung 4242 nicht belasten: unzureichende Mittel (Fehlercode: card_declined)" hilft tatsächlich. Ich markiere dies in etwa 30% der PRs. Der Test, den ich verwende: Wenn du diesen Fehler in den Produktionsprotokollen um 3 Uhr morgens sehen würdest, könntest du ihn ohne zusätzliche Protokollierung und erneutes Bereitstellen diagnostizieren?
3. Allgemeine Ausnahme anstelle spezifischer Ausnahmen abfangen. Das ist umstritten, da einige Stilrichtlinien es empfehlen, aber ich habe gesehen, dass es zu viele versteckten Fehlern führt. Wenn du die Ausnahme abfängst, fängst du auch NullPointerException, IllegalStateException und all die anderen Ausnahmen ab, die Programmierfehler anstelle von Laufzeitbedingungen anzeigen. Fange das ab, was du erwartest zu bearbeiten. Lass Programmierfehler abstürzen - so findest du sie.
4. Externe Daten an der Grenze nicht validieren. API-Antworten, Benutzereingaben, Dateiinhalte - wenn sie aus deinem Prozess kommen, validiere sie sofort. Ich sehe Entwickler, die in der Geschäftsschicht validieren, was bedeutet, dass ungültige Daten bereits durch mehrere Funktionen propagiert wurden. Dann debuggst du, warum ein Nullwert in deine Datenbank gelangte. Validieren an der Grenze, schnell scheitern, klare Fehler zurückgeben.
5. Wiederholungslogik ohne exponentielles Backoff. Der Dienst ist down, also versuchst du es sofort erneut. Er ist immer noch down, also versuchst du es sofort wieder. Herzlichen Glückwunsch, du hast soeben eine Dienstverschlechterung in einen kompletten Ausfall verwandelt, indem du ihn mit Wiederholungen bombardierst. Ich sehe dies in 15% der PRs, die Wiederholungslogik hinzufügen. Verwende immer exponentielles Backoff mit Jitter. Habe immer eine maximale Anzahl an Wiederholungen. Überlege immer, ob ein erneuter Versuch überhaupt für diese Operation sinnvoll ist.
6. Teilweise Fehler in Batch-Operationen nicht behandeln. Du bearbeitest 1.000 Datensätze. Datensatz 500 schlägt fehl. Was geschieht mit den Datensätzen 501-1000? In den meisten Codes, die ich überprüfe, werden sie nie verarbeitet. Der Batch schlägt fehl, wird erneut versucht, schlägt erneut bei Datensatz 500 fehl und du bist festgefahren. Behandle teilweise Fehler explizit: Verfolge, was erfolgreich war, was fehlgeschlagen ist und warum. Mach deine Batch-Operationen wiederaufnehmbar.
7. Annehmen, dass Datenbanktransaktionen immer erfolgreich sind. Du beginnst eine Transaktion, machst etwas Arbeit und bestätigst. Aber was, wenn die Bestätigung fehlschlägt? Was, wenn du die Datenbankverbindung mitten in der Transaktion verlierst? Ich sehe Code, der dies in 25% der PRs, die Datenbankcode berühren, nicht behandelt. Das Ergebnis: deine Anwendung...