From 1f9814486be0f3aa49e3eae854fafa1877a0b67b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 25 Jul 2026 00:01:32 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(bpp):=20nie=20raportuj=20do=20Rollbara?= =?UTF-8?q?=20spodziewanej=20wisz=C4=85cej=20referencji=20w=20=5F=5Fstr=5F?= =?UTF-8?q?=5F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Autor_Jednostka.__str__ miał już fallback na wypadek błędów "podczas usuwania" — komentarz wprost o tym mówił. Ale łapał gołe `except Exception` i raportował WSZYSTKO do Rollbara, łącznie z przypadkiem, dla którego ten fallback powstał: podczas kaskadowego kasowania Django buduje str() obiektu, który wciąż żyje w pamięci, choć jego wiersz i wiersz po drugiej stronie FK już zniknął. Efekt: itemy DoesNotExist: Autor matching query does not exist (#1098, #450) w produkcyjnym strumieniu błędów, mimo że aplikacja działała poprawnie. Rozdzielamy: ObjectDoesNotExist → log bez Rollbara (do_rollbar=False, parametr istniał już w zaloguj_polkniety_wyjatek właśnie dla benignych fallbacków); cokolwiek innego → raportuj jak dotąd. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH --- src/bpp/models/autor.py | 22 +++++++---- ...+autor-jednostka-str-wiszacy-fk.bugfix.rst | 5 +++ .../test_models/test_autor_jednostka_str.py | 39 +++++++++++++++++++ 3 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst create mode 100644 src/bpp/tests/test_models/test_autor_jednostka_str.py diff --git a/src/bpp/models/autor.py b/src/bpp/models/autor.py index 77cfef78c..f395ede78 100644 --- a/src/bpp/models/autor.py +++ b/src/bpp/models/autor.py @@ -15,7 +15,7 @@ RangeOperators, ) from django.contrib.postgres.search import SearchVectorField as VectorField -from django.core.exceptions import ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import RegexValidator from django.db import IntegrityError, models, transaction from django.db.models import CASCADE, SET_NULL, Count, Func, Q, Sum @@ -816,6 +816,9 @@ class Meta: # patrz migracja 0444_deferred_podstawowe_miejsce_pracy. def __str__(self): + komunikat = f"Budowanie reprezentacji tekstowej Autor_Jednostka (pk={self.pk})" + fallback = f"Autor_Jednostka #{self.pk if self.pk else 'nowy'}" + try: autor_str = str(self.autor) if self.autor_id else "???" jednostka_str = self.jednostka.skrot if self.jednostka_id else "???" @@ -824,13 +827,18 @@ def __str__(self): if self.funkcja_id and self.funkcja: buf = f"{autor_str} ↔ {self.funkcja.nazwa}, {jednostka_str}" return buf + except ObjectDoesNotExist: + # SPODZIEWANE, nie błąd aplikacji: podczas kaskadowego kasowania + # Django buduje str() obiektu, który wciąż żyje w pamięci, choć + # jego wiersz — i wiersz po drugiej stronie FK — już zniknął. + # Log zostaje (diagnostyka), ale do Rollbara tego nie wysyłamy, + # bo zaśmiecało to strumień błędów produkcyjnych. + zaloguj_polkniety_wyjatek(komunikat, logger=logger, do_rollbar=False) + return fallback except Exception: - zaloguj_polkniety_wyjatek( - f"Budowanie reprezentacji tekstowej Autor_Jednostka (pk={self.pk})", - logger=logger, - ) - # Fallback w przypadku jakichkolwiek błędów podczas usuwania - return f"Autor_Jednostka #{self.pk if self.pk else 'nowy'}" + # Cokolwiek innego jest naprawdę nieoczekiwane — raportuj. + zaloguj_polkniety_wyjatek(komunikat, logger=logger) + return fallback def clean(self, exclude=None): if self.rozpoczal_prace is not None and self.zakonczyl_prace is not None: diff --git a/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst b/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst new file mode 100644 index 000000000..3eb0062eb --- /dev/null +++ b/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst @@ -0,0 +1,5 @@ +Kasowanie autora nie zgłasza już do monitoringu błędów fałszywego alarmu. +Podczas kaskadowego usuwania Django buduje opis tekstowy powiązania +autor–jednostka, którego dane właśnie zniknęły; ten spodziewany przypadek +trafiał do monitoringu jako błąd aplikacji. Zachowanie samego kasowania się +nie zmienia. diff --git a/src/bpp/tests/test_models/test_autor_jednostka_str.py b/src/bpp/tests/test_models/test_autor_jednostka_str.py new file mode 100644 index 000000000..36c835a86 --- /dev/null +++ b/src/bpp/tests/test_models/test_autor_jednostka_str.py @@ -0,0 +1,39 @@ +"""Reprezentacja tekstowa ``Autor_Jednostka`` przy wiszącej referencji. + +Podczas kaskadowego kasowania autora Django potrafi zbudować ``str()`` +obiektu ``Autor_Jednostka``, który wciąż żyje w pamięci, choć jego wiersz +(i wiersz autora) już zniknął. ``__str__`` ma na to fallback i nie wywala +się — ale raportował ten spodziewany przypadek do Rollbara jako błąd +(#1098, #450). +""" + +import pytest + +from bpp.models.autor import Autor, Autor_Jednostka + + +@pytest.mark.django_db +def test_str_autor_jednostka_dziala_normalnie(autor_jan_kowalski, jednostka): + aj = Autor_Jednostka.objects.create(autor=autor_jan_kowalski, jednostka=jednostka) + + assert str(autor_jan_kowalski) in str(aj) + assert jednostka.skrot in str(aj) + + +@pytest.mark.django_db +def test_str_autor_jednostka_po_skasowaniu_autora_nie_raportuje( + autor_jan_kowalski, jednostka, mocker +): + """Wisząca referencja po kaskadzie → fallback, ale BEZ raportu do Rollbara.""" + report = mocker.patch("bpp.util.wyjatki.rollbar.report_exc_info") + + Autor_Jednostka.objects.create(autor=autor_jan_kowalski, jednostka=jednostka) + # Świeży obiekt z bazy — bez podpiętego w pamięci cache'u FK ``autor``, + # dokładnie tak jak w produkcyjnym tracebacku. + aj = Autor_Jednostka.objects.get(autor=autor_jan_kowalski, jednostka=jednostka) + Autor.objects.filter(pk=autor_jan_kowalski.pk).delete() + + assert str(aj) == f"Autor_Jednostka #{aj.pk}" + assert not report.called, ( + "Spodziewana wisząca referencja nie powinna iść do Rollbara" + ) From 4d223a336b4c400e81cfa142e79d15bcd40b8d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 25 Jul 2026 00:21:37 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(bpp):=20domknij=20self-review=20?= =?UTF-8?q?=E2=80=94=20test=20drugiej=20ga=C5=82=C4=99zi=20i=20uczciwsza?= =?UTF-8?q?=20skala?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Brakował test kierunku odwrotnego: cały sens tej zmiany to rozdzielenie dwóch gałęzi, a pokryta była jedna. Nowy test pilnuje, że wyjątek INNY niż ObjectDoesNotExist nadal trafia do Rollbara — zweryfikowany mutacją (cofnięcie `except ObjectDoesNotExist` do `except Exception` wywala test). - Dołożona asercja caplog: wyciszamy Rollbara, NIE diagnostykę. - Sprostowana skala w komentarzu i newsfragmencie. To nie był "strumień" — to 9 wystąpień w 2 itemach przez 10 dni. Prawdziwy powód zmiany jest inny i teraz jest opisany: hash itemu obejmuje numer linii, więc KAŻDY deploy zakładał nowy item i alert szedł od nowa. - Komentarz nie twierdzi już, że to "kaskadowe kasowanie" jako fakt — traceback z Rollbara nie zawiera ramek wywołującego. Wskazujemy easyaudit (object_repr liczony w transaction.on_commit) jako najpewniejszy znany mechanizm, ten sam, który opisuje komentarz przy Jednostka.__str__. - Odnotowane wprost: logger `bpp.*` nie ma dziś handlera w LOGGING, więc ślad ląduje na stderr przez logging.lastResort. Diagnostyka jest słaba, ale to osobny temat — nie powód, by zostawiać fałszywy alarm. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH --- src/bpp/models/autor.py | 22 ++++++--- ...+autor-jednostka-str-wiszacy-fk.bugfix.rst | 7 +-- .../test_models/test_autor_jednostka_str.py | 45 ++++++++++++++++--- 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/bpp/models/autor.py b/src/bpp/models/autor.py index f395ede78..9180d7ed7 100644 --- a/src/bpp/models/autor.py +++ b/src/bpp/models/autor.py @@ -828,11 +828,23 @@ def __str__(self): buf = f"{autor_str} ↔ {self.funkcja.nazwa}, {jednostka_str}" return buf except ObjectDoesNotExist: - # SPODZIEWANE, nie błąd aplikacji: podczas kaskadowego kasowania - # Django buduje str() obiektu, który wciąż żyje w pamięci, choć - # jego wiersz — i wiersz po drugiej stronie FK — już zniknął. - # Log zostaje (diagnostyka), ale do Rollbara tego nie wysyłamy, - # bo zaśmiecało to strumień błędów produkcyjnych. + # SPODZIEWANE, nie błąd aplikacji: str() bywa wołany na obiekcie, + # który wciąż żyje w pamięci, choć jego wiersz — i wiersz po + # drugiej stronie FK — już zniknął. Najpewniejszy znany nam + # wywołujący to audyt easyaudit, liczący ``object_repr`` w + # ``transaction.on_commit`` (ten sam mechanizm opisuje komentarz + # przy ``Jednostka.__str__``); traceback z Rollbara nie zawiera + # ramek wywołującego, więc nie zgadujemy dalej. + # + # Nie raportujemy tego do Rollbara: hash itemu obejmuje numer + # linii, więc KAŻDY deploy zakładał nowy item i alert szedł od + # nowa, mimo że aplikacja zachowywała się poprawnie. + # + # Uwaga: logger ``bpp.*`` nie ma dziś własnego handlera w + # ustawieniach, więc ten ślad ląduje na stderr przez + # ``logging.lastResort``. Diagnostyka jest zatem słaba — ale to + # osobny temat (konfiguracja LOGGING), nie powód, by zostawiać + # fałszywy alarm w Rollbarze. zaloguj_polkniety_wyjatek(komunikat, logger=logger, do_rollbar=False) return fallback except Exception: diff --git a/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst b/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst index 3eb0062eb..afdad9d32 100644 --- a/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst +++ b/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst @@ -1,5 +1,2 @@ -Kasowanie autora nie zgłasza już do monitoringu błędów fałszywego alarmu. -Podczas kaskadowego usuwania Django buduje opis tekstowy powiązania -autor–jednostka, którego dane właśnie zniknęły; ten spodziewany przypadek -trafiał do monitoringu jako błąd aplikacji. Zachowanie samego kasowania się -nie zmienia. +Usunięto fałszywy alarm w monitoringu błędów, zgłaszany przy kasowaniu autora. +Zachowanie samego kasowania nie ulega zmianie. diff --git a/src/bpp/tests/test_models/test_autor_jednostka_str.py b/src/bpp/tests/test_models/test_autor_jednostka_str.py index 36c835a86..c2b1b7b22 100644 --- a/src/bpp/tests/test_models/test_autor_jednostka_str.py +++ b/src/bpp/tests/test_models/test_autor_jednostka_str.py @@ -1,12 +1,18 @@ """Reprezentacja tekstowa ``Autor_Jednostka`` przy wiszącej referencji. -Podczas kaskadowego kasowania autora Django potrafi zbudować ``str()`` -obiektu ``Autor_Jednostka``, który wciąż żyje w pamięci, choć jego wiersz -(i wiersz autora) już zniknął. ``__str__`` ma na to fallback i nie wywala -się — ale raportował ten spodziewany przypadek do Rollbara jako błąd -(#1098, #450). +Po skasowaniu autora ``str()`` bywa wołany na obiekcie ``Autor_Jednostka``, +który wciąż żyje w pamięci, choć jego wiersz (i wiersz autora) już zniknął — +m.in. przez audyt ``easyaudit``, który liczy ``object_repr`` w +``transaction.on_commit`` (patrz analogiczny komentarz w +``bpp/models/jednostka.py``). ``__str__`` ma na to fallback i się nie wywala, +ale raportował ten spodziewany przypadek do Rollbara jako błąd (#1098, #450). + +Testujemy OBIE gałęzie rozdzielenia: spodziewany ``ObjectDoesNotExist`` → +log bez Rollbara, wszystko inne → raport jak dotąd. """ +import logging + import pytest from bpp.models.autor import Autor, Autor_Jednostka @@ -22,7 +28,7 @@ def test_str_autor_jednostka_dziala_normalnie(autor_jan_kowalski, jednostka): @pytest.mark.django_db def test_str_autor_jednostka_po_skasowaniu_autora_nie_raportuje( - autor_jan_kowalski, jednostka, mocker + autor_jan_kowalski, jednostka, mocker, caplog ): """Wisząca referencja po kaskadzie → fallback, ale BEZ raportu do Rollbara.""" report = mocker.patch("bpp.util.wyjatki.rollbar.report_exc_info") @@ -33,7 +39,32 @@ def test_str_autor_jednostka_po_skasowaniu_autora_nie_raportuje( aj = Autor_Jednostka.objects.get(autor=autor_jan_kowalski, jednostka=jednostka) Autor.objects.filter(pk=autor_jan_kowalski.pk).delete() - assert str(aj) == f"Autor_Jednostka #{aj.pk}" + with caplog.at_level(logging.ERROR, logger="bpp.models.autor"): + assert str(aj) == f"Autor_Jednostka #{aj.pk}" + assert not report.called, ( "Spodziewana wisząca referencja nie powinna iść do Rollbara" ) + # Wyciszamy Rollbara, NIE diagnostykę — ślad w logu musi zostać. + assert any("Autor_Jednostka" in r.message for r in caplog.records) + + +@pytest.mark.django_db +def test_str_autor_jednostka_nadal_raportuje_nieoczekiwane_bledy( + autor_jan_kowalski, jednostka, mocker +): + """Druga gałąź: cokolwiek INNEGO niż DoesNotExist nadal idzie do Rollbara. + + Bez tego testu nic nie broni przed cofnięciem całego sensu tej zmiany — + zamianą ``except ObjectDoesNotExist`` z powrotem na ``except Exception``. + """ + report = mocker.patch("bpp.util.wyjatki.rollbar.report_exc_info") + aj = Autor_Jednostka.objects.create(autor=autor_jan_kowalski, jednostka=jednostka) + + # Awaria, która NIE jest wiszącą referencją — np. uszkodzony rekord autora. + mocker.patch( + "bpp.models.autor.Autor.__str__", side_effect=RuntimeError("coś padło") + ) + + assert str(aj) == f"Autor_Jednostka #{aj.pk}" + assert report.called, "Nieoczekiwany błąd MUSI trafić do Rollbara"