Przejdź do treści

FAPI007 yield-dependency-swallows

Co robi

Opt-in. Zgłasza klauzulę except wokół yield w funkcji generatora, która ani nie rzuca wyjątku ponownie, ani nie rzuca innego:

def get_db():
    db = SessionLocal()
    try:
        yield db
    except Exception:   # reported: no raise on any path
        db.rollback()
    finally:
        db.close()

Zgłoszenie stoi na linii except. Klauzula przechodzi, gdy każda ścieżka przez nią rzuca wyjątek: raise w bloku, if / else, którego wszystkie gałęzie rzucają, with, którego ciało rzuca, albo try rzucający w finally. except, który loguje i rzuca ponownie, przechodzi, tak samo jak ten, który rzuca HTTPException lub jej podklasę.

Dlaczego to źle

Zależność z yield widzi wyjątki rzucone w endpoincie. Gdy jej blok except kończy się bez rzucenia, błąd zatrzymuje się tam: klient dostaje 500, a serwer nie ma logu przyczyny (dokumentacja FastAPI, "Dependencies with yield"). Wycofanie transakcji w bloku wygląda jak porządna obsługa błędu, dlatego ten wzorzec jest częsty w kodzie pisanym przez agenta. Ruff nie ma na to reguły.

Przykład

pyproject.toml
[tool.inwards]
layers = [{ name = "app", modules = ["app"] }]

[tool.inwards.rules]
extend-select = ["FAPI007"]
app/db.py
class Session:
    def rollback(self) -> None: ...

    def close(self) -> None: ...


def new_session() -> Session:
    return Session()
app/deps.py
from collections.abc import Iterator

from app.db import Session, new_session


def get_db() -> Iterator[Session]:
    db = new_session()
    try:
        yield db
    except Exception:
        db.rollback()
    finally:
        db.close()

inwards check app

app/deps.py:10:5: FAPI007 `except Exception` around the `yield` in `get_db` never re-raises: an error from the endpoint stops here, so the client gets a 500 and the server has no log of the cause.
  fix: Re-raise the exception at the end of the except block.
    1. Finish the except block with `raise`, after the cleanup it does (a rollback, say), so FastAPI sees the error.
    2. If the client should get another status, raise an HTTPException (`raise HTTPException(...) from exc`) instead of returning quietly.
    3. Don't drop the except block to make this finding go away: the cleanup in it is needed.
  docs: https://sircypkowskyy.github.io/inwards/rules/FAPI007/

Poprawione: wycofanie transakcji zostaje, a błąd leci dalej.

app/deps.py
from collections.abc import Iterator

from app.db import Session, new_session


def get_db() -> Iterator[Session]:
    db = new_session()
    try:
        yield db
    except Exception:
        db.rollback()
        raise
    finally:
        db.close()

inwards check app

Jak działa

FAPI007 czyta jedną funkcję naraz, bez grafu wywołań i bez modelu FastAPI. Dla każdej funkcji z własnym yield (nie takim z zagnieżdżonej funkcji) patrzy na instrukcje try, w których ciele stoi yield, i sprawdza każdą z ich klauzul except (także except*). Klauzula jest zgłaszana, chyba że jakaś instrukcja w niej rzuca wyjątek na każdej ścieżce, o ile widać to w składni.

Reguła czyta każdy generator z kodu projektu, nie tylko funkcje przekazane do Depends(...): moduł zależności często nie importuje FastAPI, a przeszukiwanie całego projektu pod kątem użycia spowolniłoby sprawdzenie w hooku edycji. Plik jest parsowany tylko wtedy, gdy zawiera zarówno yield, jak i except.

Dwa rodzaje generatorów są pomijane, bo połykanie jest w nich poprawne: te z dekoratorem contextmanager lub asynccontextmanager (menedżer kontekstu, który tłumi błąd, robi to celowo) oraz te z dekoratorem fixture (fixture testowy nigdy nie widzi wyjątku testu).

Jak naprawić

  1. Zakończ blok except słowem raise, po sprzątaniu, które wykonuje, żeby FastAPI zobaczył błąd i go zalogował.
  2. Jeśli klient ma dostać inny status, rzuć w bloku HTTPException.
  3. Nie usuwaj bloku except: jego sprzątanie (na przykład wycofanie transakcji) jest potrzebne.

Bezpieczeństwo poprawki

Brak automatycznej poprawki (autofix: false). To, czy błąd ma dotrzeć do klienta bez zmian, czy zamienić się w HTTPException, to decyzja autora.

Konfiguracja

Waga: błąd. Opt-in: włącz przez extend-select. Nie ma własnych opcji; modules działa jak w każdej regule, a check-tests jak w każdej regule treści (tabele opcji).

Każde zgłoszenie można wyciszyć w kodzie przez # inwards: ignore[FAPI007] reason="..." w linii except.

Znane ograniczenia

  • Generatory, które nie są zależnościami, też są zgłaszane, gdy mają jedno yield: zwykłe def line(): try: yield ... except ...: log(), do którego nic nie wrzuca wyjątku, nigdy nie widzi błędu endpointu. Wycisz je w kodzie albo oznacz dekoratorem, jeśli to menedżer kontekstu. Generator, który ma więcej niż jedno yield albo yield w pętli for lub while, jest strumieniem i jest pomijany, bo zależność ma jedno yield; zależność z yield w dwóch gałęziach (if x: yield a / else: yield b) jest pomijana razem z nimi. Generator wywołany wewnątrz StreamingResponse(...) lub EventSourceResponse(...) w sprawdzanych plikach też jest strumieniem i jest pomijany; dopasowanie po nazwie funkcji, bez rozwiązywania importów. Sprawdzenie po edycji widzi tylko edytowany plik, więc generator strumieniowany z innego modułu jest pomijany w pełnym sprawdzeniu, ale zgłaszany, gdy edytowany jest tylko jego plik.
  • Handler, który rzuca przez wywołanie (fail(exc), gdzie fail zawsze rzuca), nie jest dla Inwards raise, więc jest zgłaszany.
  • raise, do którego prowadzą tylko niektóre ścieżki (if x: raise), jest zgłaszany; ten na każdej gałęzi if / else przechodzi.
  • Klauzula dla wąskiego wyjątku (except KeyError:) jest sprawdzana jak każda inna: błędy innych typów przechodzą dalej, ale ten jest połykany.
  • return w klauzuli przed raise kończy ścieżkę bez rzucenia i jest zgłaszany.
  • Zmierzona na korpusie (#182, rozdział 6): 4 zgłoszenia, zanim strumienie zaczęły być pomijane, wszystkie fałszywe (zdarzenia wysyłane przez serwer w Polarze, iterator rabatów w Saleorze), i żadne po tym.

Źródła