Kde zachytávat výjimky
Školení, která pořádám 
V článku Kde zachytáváte výjimky? jsem představil jednoduchý problém aplikace v Nette Frameworku, která při získávání dat může vyvolat chybu, a ptal jsem se, jak s touto chybou naložit. Řešení nabídnutá v diskusi mě příliš nepřesvědčila a ke všem jsem měl větší nebo menší výhrady. Proto jsem problém otevřel na Poslední sobotě, kde jsme spolu s ostatními účastníky myslím dali dohromady docela slušné řešení.
Model
Na prvním místě bych chtěl uvést, že chybu není vhodné ignorovat a tvářit se, že k ní nemůže dojít. Možná je chyba nepravděpodobná, možná je příznakem nějakých větších problémů, třeba ale také ne. Možná jste se dosud nesetkali s poškozenou jednou databázovou tabulkou, možná nepoužíváte MyISAM, který je k tomu náchylný, třeba ale také někdy použijete fulltextové vyhledávání MySQL a na problém narazíte. Ať tak či tak, malá pravděpodobnost chyby by nás od jejího ošetření neměla odradit a měli bychom postupovat stejně jako u pravděpodobnějších chyb (pokud bychom třeba data získávali z externího zdroje pomocí protokolu SOAP). Před problémy bychom neměli strkat hlavu do písku.
V druhé řadě považuji za nevhodné vzniklou výjimku jednoduše poslat dál. V diskusi padl termín „prosakující abstrakce“ – vyšší vrstva by podle typu výjimky poznala, jak je implementovaná nižší vrstva, čímž by vznikla nežádoucí závislost. Název výjimky ArticleException je ale také nevhodný, protože říká spíš to, kde chyba vznikla, než k jaké chybě došlo. Lepší je tedy spíš nějaká obecná DataSourceException.
Zajímavá debata vznikla o tom, kde chybu logovat. Základní představa byla v modelu ji přeložit a ve vyšší vrstvě zpracovat a případně zalogovat. Protože ale vyšší vrstva neví nic o implementaci modelu, tak ani neví, jestli by se o chybě měl dozvědět programátor (nebo spíš správce aplikace). Pokud jde o chybu, ke které běžně dochází a o které se správce aplikace dozvědět nepotřebuje, tak ji logovat nemusí, pokud o nějakou méně běžnou chybu, tak by o ní vědět měl, aby mohl zjednat nápravu. O logování by se tedy měl postarat už model.
Jako velmi praktické se tedy ukázalo vytvoření funkce, která přistoupí k databázi a pokud dojde k chybě, tak ji zaloguje a přeloží na obecnou výjimku.
Použití chybové návratové hodnoty funkce (v souladu s ostatními funkcemi PHP by to měla být hodnota false) by znamenalo jednoduché a rychlé řešení, problém by ale nastal u funkcí vracejících pravdivostní hodnotu. Řada lidí navíc toto řešení z nějakého důvodu nepovažuje za čisté.
Prezenter
Jako velmi praktické se ukázalo vytvoření komponent pro volitelné bloky. Kromě možnosti použít tyto komponenty na více stránkách se kód také zpřehlednil a dovolil lepší zpracování chyb. Za nezanedbatelnou výhodu považuji také to, že data komponent se načítají až když se na ně narazí, což nezdržuje okamžik odeslání první části stránky, který je pro pocit rychlosti aplikace nesmírně důležitý.
Tím, že je chyba zalogovaná už v modelu, se o ní v komponentě nemusíme starat. K ignorování chyb v komponentách by možná mohla existovat nějaká anotace nebo by to dokonce mohlo být výchozí chování. Většina komponent si žije sama pro sebe a neměly by způsobit chybu celé stránky. Platí to i pro komponenty s užší vazbou na hlavní zobrazená data, třeba stránkování – chceme snad, aby chyba v komponentě stránkování způsobila nezobrazení výpisu alespoň první stránky?
Každopádně si můžeme napsat jednoduchou univerzální obálku volající funkci, která by mohla vyvolat výjimku, a tuto výjimku ignorovat.
Závěr
Troufám si tvrdit, že jsme spolu s ostatními účastníky Poslední soboty dali dohromady čisté a přitom velmi praktické řešení. To, co se při pštrosím řešení napsalo na jeden řádek, na jednom řádku zůstalo, pouze přibylo pár univerzálně použitelných utilit. Všem účastníkům debaty tedy děkuji za nápady.
Výsledek je opět k dispozici na GitHubu.
Diskuse
Michal Holub:
Pardon, ale tohle mi moc čisté nepřijde (i když možná, že to praktické je)...
1) ad nepředání výjimky dál - podle mě, když má třída určité rozhraní, jsou výjimky jeho nedílnou součástí. Vím o třídě na jaké zprávy reaguje a vím taky jak se zachová v "krizi". Znalost výjimky bych položil na úroveň znalosti metod, které jsou navenek viditelné. Řekl bys potom, že výjimka DataSourceException, nebo např. metoda getLatestArticle odhalují něco ze střev...mě to teda nepřijde.
2) Dobrá, dejme tomu, že máš dobrý důvod pro neposlání výjimky výše. Pak ale nechápu, proč teda vymýšlíte nové typy (DataSourceException), když stejně nejde výše a loguje se přímo ve své vrstvě...stejně ta serializace proběhne někde do textu a tím se typovost ztrácí...
3) Náš cvičící vždy říkal, programujte defensivně (p. Adámek na FI MUNI)...tj. vždycky očekávejte to nejhorší -> já jako programátor vyšší vrstvy se nemůžu spolehnout na to, že datová vrstva správně zareaguje na výjimku (zaloguje) datové vrstvy musím brát na vědomí, že třídu může někdo někdy vyměnit a pak se výjimka ztratí.
Nechci sypat z rukávu jak to dělat lépe, to teď po ránu nevím, ale rozhodně se mi nelíbí přístup ignorování výjimky (a prázdný blok catch prostě ignorování je)
Jakub Vrána 
:
Obávám se, že jsi to pochopil špatně.
1. Dál se nepředá výjimka DibiException, která by odhalila implementaci modelu. DataSourceException se z modelu předává.
3. Prezenter nepozná, jestli by se o chybě měl dozvědět správce aplikace, protože nezná implementaci modelu. Když nová třída nebude chyby logovat, je to její chyba. To je totéž, jako kdyby výjimku slízla, nenareportovala a třeba vrátila jen false.
Jakub Vrána : Michal Holub:
Boha jsem to ale vůl, úplně jsem to throw přehlédl (tak moc, že bych přísahal, že tam ráno nebylo :)V tom případě všechny tři mé body padají :)
v6ak:
Házet DataSourceException je sice čístší než házet DibiException, ale ve webových aplikacích ne tolik užitečné. Na druhou stranu je pravda, že není až takový problém vytvořit vrstvu, která toto vyřeší.
Místo logování:
Praktická stránka: logovat v modelu nebo v presenteru je zhruba stejně náročné.
Čistota a vedlejší následky: Má model vědět, kam logovat? A, jak tady napsal Michal Holub, proč házet výjimku, když je již logována? Extrémní by bylo logování výjimek hodit až do Dibi a logovat i porušení duplicity, které by mělo být spíše dále ošetřeno. Co když uvnitř dibi bude chyba, třeba fetchAll by za určitých okolností házelo IllegalArgumentException? BaseModel to pošle dál a TryControl to bez zalogování polkne.
Prázdný catch blok je obecně varování "pozor, něco tu je možná špatně" a měl by být používán obezřetně. Typicky u InterruptedException. Zatím jsem však nenašel čistý důvod u obecného Exception.
Model by měl výjimky, o kterých by se měl dozvědět uživatel (již se daného turnaje účastníte), mít uvedeny v rozhraní, aby s tím šlo dále pracovat a vhodně prezentovat uživateli. Proto by měl v takovém případě obecné DibiException (nebo DataSourceException v případě Tvého BaseModelu) prozkoumat, zde třeba na chybová kód znamenající porušení duplicity, a případně přeložit. Vše, s čím presenter nepočítá, má být zachyceno a logováno. Naopak, DibiException udávající duplicitu zde být logována nemá. Nebo sis chyby "ke které běžně dochází a o které se správce aplikace dozvědět nepotřebuje" představoval jinak?
Metodu renderBests jsem asi moc nepochopil. Jaký má význam, pokud to řeší komponenta?
Ve třídě TryControl by se hodila deklarace abstraktní metody pro včasné objevení chyby.
V zájmu jednoduchosti kódu může být někdy přínosné nemuset vytvářet novou třídu, protože PHP neumí anonymní třídy. V případě http://github.com/v6ak/mvc-exception-demo/…/ArticlePresenter.php to je splněno. U Tvého řešení by to šlo vyřešit v PHP 5.3 pomocí anonymní funkce, případně méně elegantně pomocí callbacku array($this, 'foo') v 5.2. Uvedení šablony přímo v kódu je ve Tvém případě asi nutností (vzhledem k současným možnostem Nette šablon), u mého řešení ne. Byl bych rád, kdyby bylo možno oboje - podle složitosti by si programátor mohl vybrat. Ale to by už chtělo vrtání do vnitřností Nette.
Jinak s nekritickými částmi z principu souhlasím.
v6ak: Jakub Vrána :
Myslím si, že model by rozhodně měl vědět, kam a jak logovat.
Máš pravdu, že zachytávat obecné Exception je chyba. Opravil jsem to. Abstraktní metodu jsem taky přidal.
Co se chyby duplicity týče, tak to je podle mě přesně chyba, kterou model logovat nemá a má ji poslat dál (pod obecným názvem).
Metoda renderBests je jen ukázka toho, jak by se řešila samostatná stránka s výběrem článků, která má v případě chyby skončit pětistovkou. Byla zmíněna v textu zadání.
Kde bych potřeboval anonymní třídu? Pokud máš na mysli BestArticlesComponent, tak o tom jsme se na Poslední sobotě taky bavili, že by mohla být nějaká obecná třída přijímající callback, přesně jak píšeš. Většinou jsou ale komponenty o něco složitější.
„Uvedením šablony přímo v kódu“ myslíš název souboru, který šablonu obsahuje?
Jakub Vrána : v6ak:
Nevím přesně co, ale něco se mi na místě logování tady nezdá. Asi jediný rozumný důvod pro logování v modelu je oddělení nedůležitých výjimak od důležitých. Ale, na druhou stranu, to by mohl udělat už model a vlastní logování ponechat dál. Nenutilo by to ošklivě polykat výjimky nebo vracet nesmyslné hodnoty. Ostatně, při čtení kódu patří prázdný catch blok k velkým poutačům pozornosti. Proto při psaní takového bloku vždy uvádím důvod, proč je prázdný. A taky se snažím, aby jich prázdnách bylo co nejméně.
K anonymní třídě: chápeš to dobře. A podobně to myslím se šablonou. V šabloně by byl i kód (ne název souboru) šablony pro komponentu. Asi je lepší, když tu jsou obě možnosti (oddělený soubor i uvedení přímo v kódu) a člověk si může vybrat. U menších věcí je vytvoření nového souboru zbytečná režie, u větších to naopak přispívá k čitelnosti.
v6ak: Jiří Knesl:
Jakube, mě se to řešení moc nelíbí.
1) definice BaseModelu jen proto, abych měl k dispozici nějakou společnou funkcionalitu. To je ukázka špatného použití dědičnosti. Takhle se zbytečně hromadí funkce v abstraktních třídách jen proto, že jsou potřeba potomky. A co když budu chtít podobnou funkcionalitu použít i mimo? To vede ke copy-paste (viditelné např v TryControlu). Když už bych chtěl try...catch blok skrýt, tak bych tu funkcionalitu nabídl zvláštní třídou.
2) Model je ekvivalentní tomu, co mi vychází po vygenerování kódu z class-diagramu. Není správné svévolně přidávat metody jen proto, že je použije persistence (model a persistence není totéž). Třída nabízející tryFetchAll by měla být pojmenovaná tak, aby bylo jasné, že se vztahuje k dibi, ne k objektu modelu.
3) BaseModel i TryControl zakrývají existenci chyby, to nevede ke korektnímu chování ze 3 důvodů:
a) programátor by měl vědět, že došlo k chybě - měl by být "vidět" kód algoritmu, ale i kód ošetření. Pokud možno z důvodu čitelnosti každé ve zvláštní metodě (což lze v PHP udělat jen vyjímkami), ale v téže třídě (pokud není obojí příliš složité).
b) implementace "nižší vrstvy" (ve skutečnosti MVP nejsou vrstvy, vícevrstvá architektura je něco jiného) může být samozřejmě známa vyšší vrstvě - a měla by, pokud jsou součástí stejného balíčku, tedy např.balíčku FrontModule\Controls\BestArticles.
c) ignorace chyb nevede k robustnosti, ale k neočekávaným a těžko odhalitelným chybám. Logování jako takové je nedostatečné, to je jen diagnostika, systém potřebuje vědět, že se něco děje a proto si myslím, že v těch prázdných catch blocích by mělo být nejen zalogování, pokus o korektní zpracování toho, co zpracovat umím, ale znovuvyhození vyjímky, pokud se objeví situace, kterou ta daná třída řešit neumí (nebo ani umět nemá).
4) " chceme snad, aby chyba v komponentě stránkování způsobila nezobrazení výpisu alespoň první stránky?" - ano, chceme, na předem zjistitelnou chybu se přijde už pomocí Selenia apod. před deploymentem. Pokud se chyba objeví až na serveru (což se u otestovaného kódu v praxi občas stává), musí se na ni přijít co nejrychleji. A 500 je sakra viditelnější než to, že někde vespod stránky chybí 150 bajtů HTML. Samozřejmě je zde kritérium, že každý kód má osobu, která se o něj stará a příští pracovní den problém vyřeší.
Jinak jsem rád, že si tuhle otázku otevřel, téma řešení chyb je hodně zajímavé a složité. Za poslední dny jsem díky tomu našel nějaké odkazy, které by možná lidé ocenili:
1) http://blogs.msdn.com/b/oldnewthing/archive/…/352949.aspx (krátký blogpost o rozdílu kontrolování chybových kódů a vyhazování vyjímek)
2) http://www.joelonsoftware.com/articles/Wrong.html (plno užitečných tipů, jak předcházet chybám)
3) http://developer.apple.com/mac/library/documentation/…-CH203-BAJCBIFC (zpracování chyb v Objective C a Cocoa, které má sice vyjímky, ale používá je velmi zřídka, má chybové kódy a pro mě nový mechanismus ErrorResponderů, který vzdáleně připomíná vyhazování vyjímek)
4) sekce o zpracování chyb a práci s vyjímkami v knize Čistý kód
Jakub Vrána :
4. Tak na tom se rozhodně neshodneme, navíc to odporuje zadání, kde bylo explicitně uvedeno, že chyba v komponentě nesmí znemožnit zobrazení stránky. Chybami programátorů by neměli být otravování běžní uživatelé. Argument, že když začnou volat zákazníci s tím, že jim nejedou stránky, tak problém spíš někdo začne řešit, je smutný. Způsob hlášení chyb by měl být natolik naléhavý, aby tento mezičlánek v podobě uživatelů nepotřeboval – nejsou to přece naši rukojmí.
Jakub Vrána : Jiří Knesl:
Volat zákazníci nemusí, zodpovědné osobě má běžet monitoring serveru. Nicméně je škoda, že jsi odpověděl na ten nejméně důležitý bod mé odpovědi (zejména pokud je to mimo zadání, ergo ten můj bod se dal klidně ignorovat).
Jakub Vrána :
Pak ale aplikace nemá co vyhazovat pětistovku a se správcem aplikace má komunikovat nezávislým kanálem.
Co se prvních dvou bodů týče, tak nevidím žádné místo náchylné ke copy–paste. Metoda tryFetchAll vznikla tak, že jsem nadhodil „A co kdyby existovalo nějaké dibi::tryFetchAll?“ Ale protože taková metoda nebyla, tak jsme ji šoupli do modelu. Ale vzhledem k tomu, že vyvolává chybu specifickou pro model, tak mi tam vlastně docela pasuje.
3. BaseModel žádnou chybu nezakrývá, pouze ji loguje a překládá na jinou. TryControl chybu skrývá a je to jeho hlavní náplň práce. Pokud by se této funkčnosti dosahovalo třeba anotací, tak by to bylo asi šikovnější, to jsem ostatně zmínil i v článku. Ale je to přesně ta funkčnost, kterou potřebuji – když dojde k chybě v komponentě, tak reakce podle zadání je ta, že se komponenta nezobrazí.
Jakub Vrána : Jiří Knesl:
(budu dál číslovat, ať se dá nějak odkazovat)
No pokud je ve specifikaci, že nefunkčnost stránkování nevyřadí aplikaci (viděl jsem už pěkných pár specifikací, ale tohle fakt ještě ne), je tento bod bezpředmětný.
5) Co se komunikace nezávislým kanálem týká, tady mluvím ze zkušenosti. Logování se používá buď málo, nebo tolik, že se tím nedá probrat, nikdo ty logy nekontroluje. Zato monitoring v nějaké podobě aktivně používaly snad všechny firmy, s kterými jsem měl co dočinění. Mnohdy logy nečetli programátoři, ikdyž věděli, že je tam nějaká chyba, kdeždo třeba výstražné SMS z monitoringu chodily přímo jednateli firmy - to je zásadní rozdíl.
6) Obecně si myslím, že je špatně šoupat to do modelu. Jedná se přeci o API dibi, ne o API toho modelu.
7) U toho controlu bych taky nepoužil ani dědičnost, ani anotaci. Jednoduše bych dal kód přímo do toho Controlu. Proč? Je to přeci jeho specifikace. Pokud by to bylo požadováno u více controlů, tak bych to řešil nějakou externí třídou, nikdy ne dědičností (taky bys pak mohl mít po nutnosti přidání podpory AJAXu abstraktní třídy: TryControl, AJAXControl, TryAJAXControl - a nebo by ses smířil s obecnou třídou, která by nabízela API i pro AJAX/popř.Try, ikdyž má umět třeba jen jedno z toho - což já považuju za nečisté a do budocna obtížně udržovatelné řešení).
8) s naším tématem to moc nesouvisí, ale hned po duplicitním kódu je dědičnost druhá nejhorší věc, pokud chci udělat kód neudržovatelný, třídy přerostlé a chování nedokumentovatelné.
Jakub Vrána :
5. Mám za to, že Nette\Debug::enable() s uvedením e-mailu se používá celkem běžně.
6. Klíčové mi přijde to, že to vyhazuje výjimku specifickou pro model. Tudíž to nemůže být součástí Dibi.
7. Uveď prosím tu externí třídu a způsob jejího volání. Pořád se mi anotace zdá ideální.
Jakub Vrána : martin:
zaujimave, ze logovanie chyb ste dali na miesto, kde sa chyba prebaluje a nie tam, kde sa zpracovava. to mi hlava nebere. Jakub Vrána :
V článku je to podrobně vysvětlené.
Jakub Vrána : martin:
ano, vysvetlene to je, ale:"Protože ale vyšší vrstva neví nic o implementaci modelu, tak ani neví, jestli by se o chybě měl dozvědět programátor" - to by sa snad mal dozvediet o kazdej chybe, nie?
"Pokud jde o chybu, ke které běžně dochází a o které se správce aplikace dozvědět nepotřebuje, tak ji logovat nemusí" - potom to ale nie je chyba ale predpokladany stav, ktory, ak by si vobec vynimku zasluzil, tak nejaku konkretnu a nie DataSourceException
Jakub Vrána :
O každé chybě se rozhodně dozvědět nemusí. Pokud komunikujeme s nespolehlivým serverem, u kterého víme, že občas nemusí odpovědět a s čímž hlavně nemůžeme nic udělat, tak tuto chybu hlásit nemusíme.
Dobrou ukázkou je třeba box s Twitterem, který chceme zobrazit na svých stránkách. Co když Twitter nepojede? Pošleme si report a co bychom s ním dál dělali?
Jakub Vrána : martin:
- pokial si informacie o tom, ze twitter nejde nebudete reportovat vobec, tak mozete prist o report o chybe, ktora nemusi byt sposobena nespolahlivostou twitteru (napr. zle pouzite url alebo cokolvek podobne) - to znamena, ze tam ten box nebude nikdyeste aby som doplnil to, co som sa snazil naznacit v predchadzajucom prispevku na priklade twitteru:
- model vacsinou nevie, na co sa data, ktore z twitteru nacital, pouziju. takze ak na nacitani dat z twitteru stoji hlavna prezentacia stranky, asi by bolo dobre tu chybu zalogovat. ak len box, tak zalogovat s uplne inou urovnou dolezitosti napriklad. co je ale dolezite, tak model vobec nema tusenie, aka je ta chyba. preto je nezmysel na jeho urovni rozhodovat o tom, ci nieco zalogovat alebo nie.
- specialna chyba by sa mohla napriklad vracat na to, aby mohol system zobrazit predtym nacachovany stav stiahnuty z twitteru, pokial tento nie je dostupny
Jakub Vrána :
Ale jistěže model ví, k jaké došlo k chybě a jestli by se o ní někdo měl dozvědět. Kdo jiný než model by o tom měl rozhodovat, když nikdo neví, co vlastně model dělá!
Model nicméně chybu pošle dál (pod jiným názvem), takže si ji dál může zpracovat i volající. Pokud slouží ke zobrazení volitelného boxu, tak s ní dál nic dělat nemusíme (BestArticlesComponent). Pokud naopak ke kritické operaci (ArticlePresenter::renderBests), tak ji opět zalogujeme. V logu tak budou dvě věci – že se nám pokazila databáze a že se kvůli tomu nezobrazuje nějaká stránka. Teoreticky by každá z těchto dvou chyb mohla chodit někomu jinému.
Jakub Vrána : martin:
"Ale jistěže model ví, k jaké došlo k chybě a jestli by se o ní někdo měl dozvědět." - to je prave ten problem - toto model nevie, model poskytuje data a funkcionalitu . ale nevie, k akym ucelom budu pouzite. takze nema tusenie, ako dolezita je ta chyba. schvalne, aku chybu/ak vobec/s akou dolezitostou by si zalogoval/nezalogoval v modeli u toho twitteru?ako si myslel to, ze nikto nevie, co model dela - ten, kto funkciu modelu vola vie, co dela. jedine, co nevie je, ako to dela. ale to nie je podstatne v tomto pripade.
Jakub Vrána :
Právě že způsob, jak to model dělá, je pro rozhodnutí o informování o chybě kritický. Když získává data z databáze, je na jeho zodpovědnosti, aby upozornil správce databáze, že je něco v nepořádku. Když by získával data z Twitteru, tak by upozornil správce Twitteru. To ale vyjde nastejno, jako kdyby poslal upozornění do zdi, takže v takovém případě nemusí dělat nic.
Jakub Vrána : martin:
spravcom databazy myslis toho, kto pisal tu vrstvu modelu pristupujucu k db, alebo chces kontaktovat fakt spravcu databazy, ktory pravdepodobne s mnozstvom tych chyb nic neurobi?a ako to bude teda s logovanim twitteru?
wdolek:
tato diskuse mi nejak unikla (na PS jsem prisel pozde :'( ) nicmene bych si dovolil navrhnout dalsi jednoduche reseni :)pouzivat PHPDcoc a anotaci @throws ;) ... programator, ktery pak ziska nejake cizi kodecky bude rovnou pri pohledu na funkci vedet, co vsecko se muze pokazit a hlavne jak se to projevi.
... a je vymalovano, ne? (jinak bych skutecne vse nechaval az na posledni - nejvyssi vrstve - s tim, ze samozrejme modelova trida na vyjimku nejak zareaguje (napriklad posle rollback) ale vyjimku posle pote dal)
Mastodont:
V Symfony akce jednoduše vrátí konstantu vyjadřující průběh operace, případně název vlastního view a podle toho je vybrána šablona. Není nutné předávat žádné výjimky. jednoduché a čisté řešení.Obdobně by bylo možné předávat chybové údaje jako další prvek vráceného pole nebo datového objektu.
Diskuse je zrušena z důvodu spamu.
© 2005-2024 