Methode

Die Analyse basiert auf den 8.000 beliebtesten PyPI-Paketen, gemessen an der Anzahl der Downloads in den letzten 30 Tagen. Sie wurden am 17.-18. Oktober heruntergeladen, mit einem Skript von Guido van Rossum, das wiederum auf Hugo van Kemenades Tool basiert, das eine Liste der beliebtesten Pakete erstellt.

Nach dem Download wurde ein zweites Skript verwendet, um für jede Datei einen AST zu erstellen und ihn zu durchlaufen, um break-, continue- und return-Anweisungen zu identifizieren, die sich direkt innerhalb eines finally-Blocks befinden.

Anschließend habe ich den aktuellen Quellcode für jedes Vorkommen gefunden und ihn kategorisiert. Für Fälle, in denen der Code falsch zu sein schien, habe ich ein Issue im Bugtracker des Projekts erstellt. Die Antworten auf diese Issues sind ebenfalls Teil der in dieser Untersuchung gesammelten Daten.

Ergebnisse

Ich habe beschlossen, keine Liste der falschen Verwendungen aufzunehmen, aus Sorge, dass dies den Bericht wie eine beschämende Übung aussehen lassen würde. Stattdessen werde ich die Ergebnisse allgemein beschreiben, aber erwähnen, dass einige der gefundenen Probleme in sehr beliebten Bibliotheken auftreten, darunter eine Cloud-Sicherheitsanwendung. Für diejenigen, die dazu neigen, sollte es nicht schwer sein, meine Analyse zu reproduzieren, da ich Links zu den von mir verwendeten Skripten im Abschnitt "Methode" bereitgestellt habe.

Die untersuchten Projekte enthielten insgesamt 120.964.221 Zeilen Python-Code, und darin fand das Skript 203 Instanzen von Kontrollflussanweisungen in einem finally-Block. Die meisten waren return, eine Handvoll waren break, und keine waren continue. Von diesen

• 46 sind korrekt und treten in Tests auf, die auf dieses Muster als Funktion abzielen (z.B. Tests für Linter, die es erkennen).
• 8 scheinen korrekt zu sein – entweder schlucken sie absichtlich Ausnahmen oder treten auf, wo keine aktive Ausnahme auftreten kann. Trotz ihrer Korrektheit ist es nicht schwer, sie umzuschreiben, um das schlechte Muster zu vermeiden, und es würde den Code klarer machen: Das bewusste Schlucken von Ausnahmen kann expliziter mit except BaseException: erfolgen, und ein return, das keine Ausnahmen schluckt, kann nach dem finally-Block verschoben werden.
• 149 waren eindeutig falsch und können zu unbeabsichtigtem Schlucken von Ausnahmen führen. Diese werden im nächsten Abschnitt analysiert.

Die Fehlerfälle

Viele der Fehlerfälle folgten diesem Muster

try:
    ...
except SomeSpecificError:
    ...
except Exception:
    logger.log(...)
finally:
    return some_value

Code wie dieser ist offensichtlich falsch, weil er absichtlich Exception-Unterklassen protokolliert und schluckt, während er BaseExceptions stillschweigend schluckt. Die Absicht ist hier entweder, dass BaseExceptions weiterpropagiert werden, oder (falls der Autor sich des BaseException-Problems nicht bewusst ist), alle Ausnahmen zu protokollieren und zu schlucken. Selbst wenn das except Exception jedoch zu except BaseException geändert würde, hätte dieser Code immer noch das Problem, dass der finally-Block alle Ausnahmen schluckt, die aus dem except-Block ausgelöst werden, und dies ist wahrscheinlich nicht die Absicht (wenn doch, kann dies mit einem weiteren try-except BaseException explizit gemacht werden).

Eine weitere Variante des Problems, das in echtem Code gefunden wurde, sieht so aus

    try:
        ...
    except:
        return NotImplemented
    finally:
        return some_value

Hier scheint die Absicht zu sein, NotImplemented zurückzugeben, wenn eine Ausnahme ausgelöst wird, aber der return im finally-Block würde den im except-Block überschreiben.

Autorreaktionen

Von den 149 fehlerhaften Instanzen von return oder break in einer finally-Klausel waren 27 veraltet, in dem Sinne, dass sie nicht im Haupt-/Master-Branch der Bibliothek vorkommen, da der Code inzwischen gelöscht oder korrigiert wurde. Die restlichen 122 befinden sich in 73 verschiedenen Paketen, und ich habe in jedem ein Issue erstellt, um die Autoren auf die Probleme aufmerksam zu machen. Innerhalb von zwei Wochen erhielten 40 der 73 Issues eine Reaktion von den Code-Betreuern

• 15 Issues hatten einen PR geöffnet, um das Problem zu beheben.
• 20 erhielten Reaktionen, die das Problem als etwas anerkannten, das es wert ist, untersucht zu werden.
• 3 schlossen das Issue als "funktioniert wie vorgesehen", da der Code nicht mehr gewartet wird, daher wird er nicht behoben.
• 2 schlossen das Issue als "funktioniert wie vorgesehen", einer sagte, dass sie alle Ausnahmen schlucken wollen, aber der andere schien sich des Unterschieds zwischen Exception und BaseException nicht bewusst zu sein.

Ein Issue wurde mit einem bereits bestehenden offenen Issue über mangelnde Reaktion auf Strg+C verknüpft, was eine Verbindung vermuten lässt.

Zwei der Issues wurden als "good first issue" gekennzeichnet.

Die korrekten Verwendungen

Die 8 Fälle, in denen die Funktion korrekt zu sein scheint (in Nicht-Test-Code), verdienen ebenfalls Aufmerksamkeit. Diese stellen den "Umbruch" dar, der durch die Sperrung der Funktion entstehen würde, denn hier muss funktionierender Code geändert werden. Ich habe die Autoren in diesen Fällen nicht kontaktiert, daher müssen wir die Schwierigkeit, diese Änderungen selbst vorzunehmen, einschätzen. In dem vollständigen Bericht ist gezeigt, dass die erforderliche Änderung in jedem Fall gering ist.

Diskussion

Das Erste, was zu beachten ist, ist, dass return/break/continue in einem finally-Block nichts ist, was wir oft sehen: 203 Instanzen in über 120 Millionen Codezeilen. Das ist möglicherweise den Lintern zu verdanken, die davor warnen.

Die zweite Beobachtung ist, dass die meisten Verwendungen falsch waren: 73% in unserer Stichprobe (149 von 203).

Schließlich waren die Autorenreaktionen überwältigend positiv. Von den 40 innerhalb von zwei Wochen erhaltenen Antworten erkannten 35 das Problem an, und 15 davon erstellten sogar einen PR, um es zu beheben. Nur zwei hielten den Code für in Ordnung, und drei erklärten, dass der Code nicht mehr gewartet wird, daher würden sie ihn nicht überprüfen.

Die 8 Instanzen, in denen der Code anscheinend wie beabsichtigt funktioniert, lassen sich nicht schwer umschreiben.