Ostatnim razem omawialiśmy problemy z zakresu formatowania, dziś zajmiemy się wyjątkami.

ErrorProne.NET odziedziczył wiele funkcji z innego mojego projektu - ExceptionAnalyzer, lecz w trochę innej formie. Oto reguły, które zawiera:

• ERP021 - Incorrect exception propagation: <nieprawidłowe> rzucenie wyjątku za pomocą throwex; zamiast throw;
• ERP022 - Unobserved exception in generic exception handler: gdy blok catch, który przechwyca wszystkie wyjątki, zwraca handlery bez próby <obserwacji> przechwyconego wyjątku.
• ERP023 - Suspicious exception handling: only Message property was observed: gdy uogólniony blok catch odsyła tylko do atrybutu wyjątku ex.Message.

Przyjrzyjmy się dokładniej tym wyjątkom.

Incorrect exception propagation

Nie ma sensu zatrzymywać się na tym problemie zbyt długo. Ten, kto przeszedł przynajmniej jedną rozmowę kwalifikacyjną albo pracował w zespole składającym się z przynajmniej dwóch osób, zapewne wie, na czym polega różnica między throw ex; a throw;



Nie ma tu żadnych niespodzianek. throw ex; <złamie> stos wywołań pierwotnego wyjątku, w wyniku czego trudno będzie przeanalizować logi i zrozumieć, co poszło nie tak. Ponieważ to znany problem, podobne błędy są dość rzadko spotykane w praktyce. Mimo wszystko, znalazłem jeden przykład wątpliwego rzucenia wyjątków w kodzie Roslyn: CompilationWithAnalyzer.cs:602.

Unobserved exception in generic exception handler

Ponieważ koncepcja tego narzędzia polega na wyszukiwaniu najbardziej podejrzanych miejsc, w większości reguły związane z przetwarzaniem wyjątków są proste, konkretne i niezbyt dokuczliwe. I tak na przykład, mimo iż puryści najchętniej zabroniliby użycia wszystkich bloków catch(Exception) w swoim kodzie, w praktyce trudno będzie to zrobić ze względu na ich powszechne występowanie. Jednak, IMHO, można i trzeba ostrzegać, gdy takie bloki zwracają handlery, nawet nie zadając sobie trudu, aby sprawdzić przechwycony wyjątek.

Dlatego poniższy kod powoduje ostrzeżenie:



Każde nawiązanie do zmiennej e w bloku catch (typu e.Message, ProcessExcpetion(e), itp.) <deaktywuje> tę regułę. Z jakiegoś powodu myślałem, że ta reguła nie powinna dawać o sobie znać zbyt często i byłem zaskoczony, że w kodzie Roslyn została uruchomiona ponad 60 (!) razy. Na początek kilka przykładów:

• AssemblyIdentityUtils.cs:40
• PdbWriter.cs:796
• AssemblyIdentity.DisplayName.cs:842
• AnalyzerFileReference.cs:158
• AbstractAnalyzerAssemblyLoader.cs:102

Tak, ta sama reguła będzie uruchamiana tylko w przypadku bloków catch {}, catch(System.Exception) oraz catch(System.AggregateException). Dla bardziej specyficznych wyjątków całkiem normalne jest zwracanie handlera bez odesłania do obiektu wyjątku.

Suspicious exception handling: only Message property was observed

Co może być gorsze od połykania wyjątków? Zapisywanie niepełnej informacji o wyjątku! Za każdym razem, gdy piszesz tylko ex.Message do logu plika, wszelkie niewiadome siły wyższe sprawiają, żeby Twój kod upadł w produkcji z TypeLoadException i obudzili Cię o 2 w nocy, aby dowiedzieć się, co poszło nie tak z kompilacją, która świetnie się sprawdziła na lokalnym pececie. A Ty, w półśnie, będziesz patrzył na log, rozmyślał nad cudownymi wpisami typu: <Exception has been thrown by the target of an invocation.>, <The type initializer for 'MyAwesomeType' threw an exception.> czy <One or more errors occurred.> i zastanawiał się, "Co się, do cholery, zdarzyło!>.

Owszem, można powiedzieć, że starannie przeglądamy kod, że nasze konstruktory statyczne nie przegapiają wyłączeń, że nie przyznajemy TPL, i ogólnie piszemy kod od razu i bez błędów. W odpowiedzi zwykle podaję przykład, który udowadnia, że wyłączenia występują nawet tam, gdzie większość ludzi zupełnie się tego nie spodziewa:



Ponieważ ograniczenie new() skutkuje użyciem Activator.CreateInstance, każde wyłączenie w konstruktorze obiektu tworzonego za pomocą uogólnionych metod w rodzaju Create<T> doprowadzi do przekształcenia oryginalnego wyłączenia w TargetInvocationExcpetion. Coś takiego łatwo pominąć podczas przeglądu, jednak znacznie łatwiej byłoby opakowywać każdy kod, który pisze tylko komunikat o wyłączeniu i nie pokazuje danych stosu wywołań + wyłączeń wewnętrznych.

I znów byłem zdziwiony ilością naruszeń tej reguły w kodzie Roslyn. Kilkanaście naruszeń. Większość z nich to oczywiste literówki, coś w tym rodzaju, gdy podczas generacji nowego wyłączenia wyraźnie zapomniano o zdefiniowaniu starego wyłączenia jako wbudowanego.

Sergey Teplyakov
Specjalista w dziedzinach .Net, C++ i Architektury aplikacji