Try / Catch / Log / Rethrow - Is Anti Pattern?

19

Widzę kilka postów, w których podkreślono znaczenie obsługi wyjątku w centralnej lokalizacji lub na granicy procesu jako dobrej praktyki zamiast zaśmiecania każdego bloku kodu wokół try / catch. Mocno wierzę, że większość z nas rozumie znaczenie tego, jednak widzę, że ludzie nadal kończą się anty-wzorcem catch-log-rethrow, głównie dlatego, że aby ułatwić rozwiązywanie problemów podczas wyjątku, chcą rejestrować informacje bardziej specyficzne dla kontekstu (przykład: parametry metody przeszedł), a sposób polega na owinięciu metody wokół try / catch / log / rethrow.

public static bool DoOperation(int num1, int num2)
{
    try
    {
        /* do some work with num1 and num2 */
    }
    catch (Exception ex)
    {
        logger.log("error occured while number 1 = {num1} and number 2 = {num2}"); 
        throw;
    }
}

Czy istnieje właściwy sposób na osiągnięcie tego, przy jednoczesnym zachowaniu dobrej praktyki obsługi wyjątków? Słyszałem o platformie AOP, takiej jak PostSharp, ale chciałbym wiedzieć, czy z tymi ramami AOP wiąże się jakikolwiek spadek lub poważny koszt wydajności.

Dzięki!

rahulaga_dev
źródło
6
Istnieje ogromna różnica między zawijaniem każdej metody w try / catch, rejestrowaniem wyjątku i pozwalaniem kodowi na poprawianie. I spróbuj / wyłap i powtórz wyjątek z dodatkowymi informacjami. Pierwsza to okropna praktyka. Drugi to doskonały sposób na poprawę jakości debugowania.
Euforia
Mówię, spróbuj / złap każdą metodę i po prostu zaloguj się, blokuj i ponownie wyślij - czy to jest w porządku?
rahulaga_dev
2
Jak zauważył Amon. Jeśli twój język ma ślady stosu, logowanie każdego połowu jest bezcelowe. Ale owijanie wyjątku i dodawanie dodatkowych informacji to dobra praktyka.
Euforia
1
Zobacz odpowiedź @ Liath. Każda udzielona przeze mnie odpowiedź odzwierciedlałaby jego odpowiedź: wychwytuj wyjątki tak wcześnie, jak to możliwe, a jeśli wszystko, co możesz zrobić na tym etapie, to zapisać jakieś przydatne informacje, zrób to i rzuć ponownie. Moim zdaniem postrzeganie tego jako antypattern jest bezsensowne.
David Arno,
1
Liath: dodano mały fragment kodu. Używam c #
rahulaga_dev

Odpowiedzi:

19

Problemem nie jest lokalny blok catch, problemem jest dziennik i ponowne rzucanie . Obsługuj wyjątek lub zawiń go nowym wyjątkiem, który dodaje dodatkowy kontekst i rzuca go. W przeciwnym razie wystąpi kilka zduplikowanych wpisów dziennika dla tego samego wyjątku.

Chodzi tutaj o zwiększenie możliwości debugowania aplikacji.

Przykład 1: Poradzić sobie

try
{
    doSomething();
}
catch (Exception e)
{
    log.Info("Couldn't do something", e);
    doSomethingElse();
}

Jeśli obsłużysz wyjątek, możesz łatwo obniżyć ważność wpisu dziennika wyjątków i nie ma powodu, aby przenosić ten wyjątek w górę łańcucha. Zostało to już rozwiązane.

Obsługa wyjątku może obejmować informowanie użytkowników o wystąpieniu problemu, rejestrowanie zdarzenia lub po prostu ignorowanie go.

UWAGA: jeśli celowo zignorujesz wyjątek, zalecamy umieszczenie komentarza w pustej klauzuli catch, który wyraźnie wskazuje, dlaczego. Dzięki temu przyszli opiekunowie wiedzą, że nie był to błąd ani leniwe programowanie. Przykład:

try
{
    context.DrawLine(x1,y1, x2,y2);
}
catch (OutOfMemoryException)
{
    // WinForms throws OutOfMemory if the figure you are attempting to
    // draw takes up less than one pixel (true story)
}

Przykład 2: Dodaj dodatkowy kontekst i rzuć

try
{
    doSomething(line);
}
catch (Exception e)
{
    throw new MyApplicationException(filename, line, e);
}

Dodanie dodatkowego kontekstu (takiego jak numer linii i nazwa pliku w kodzie parsującym) może pomóc w poprawie możliwości debugowania plików wejściowych - zakładając, że problem istnieje. Jest to rodzaj szczególnego przypadku, więc ponowne zawinięcie wyjątku w „ApplicationException” tylko w celu zmiany marki nie pomaga w debugowaniu. Upewnij się, że dodajesz dodatkowe informacje.

Przykład 3: Nie rób nic z wyjątkiem

try
{
    doSomething();
}
finally
{
   // cleanup resources but let the exception percolate
}

W tym ostatnim przypadku po prostu pozwalasz wyjściu wyjątkowi, nie dotykając go. Moduł obsługi wyjątków w najbardziej zewnętrznej warstwie może obsłużyć rejestrowanie. Ta finallyklauzula służy do upewnienia się, że wszystkie zasoby potrzebne twojej metodzie zostały wyczyszczone, ale nie jest to miejsce do rejestrowania, że ​​wyjątek został zgłoszony.

Berin Loritsch
źródło
Podobało mi się: „ Problemem nie jest lokalny blok przechwytywania, problem polega na logowaniu i ponownym rzucaniu ”. Myślę, że to ma sens, aby zapewnić czystsze rejestrowanie. Ale ostatecznie oznacza to także, że OK jest OK, ponieważ try / catch są rozproszone po wszystkich metodach, prawda? Myślę, że muszą istnieć pewne wytyczne, aby zapewnić, że praktyka ta jest przestrzegana rozsądnie, a nie każda metoda.
rahulaga_dev
W swojej odpowiedzi podałem wytyczne. Czy to nie odpowiada na twoje pytania?
Berin Loritsch,
@rahulaga_dev Nie sądzę, że istnieje wytyczna / srebrna kula, więc rozwiąż ten problem, ponieważ w dużej mierze zależy on od kontekstu. Nie może istnieć ogólna wskazówka, która wskazywałaby, gdzie należy obsłużyć wyjątek lub kiedy go ponownie wprowadzić. IMO, jedyną wskazówką, jaką widzę, jest odroczenie rejestrowania / obsługi do najnowszego możliwego czasu i unikanie logowania kodu wielokrotnego użytku, aby nie tworzyć niepotrzebnych zależności. Użytkownicy Twojego kodu nie byliby zbyt rozbawieni, gdybyś logował rzeczy (tj. Obsługiwał wyjątki) bez umożliwienia im obsługi ich na swój własny sposób. Tylko moje dwa centy :)
andreee
7

Nie wierzę, że lokalne połowy są anty-wzorcem, w rzeczywistości, jeśli dobrze pamiętam, faktycznie są egzekwowane w Javie!

Najważniejsze dla mnie przy wdrażaniu obsługi błędów jest ogólna strategia. Możesz potrzebować filtra, który wychwytuje wszystkie wyjątki na granicy usługi, możesz chcieć je ręcznie przechwycić - oba są w porządku, o ile istnieje ogólna strategia, która będzie zgodna ze standardami kodowania twoich zespołów.

Osobiście lubię wychwytywać błędy wewnątrz funkcji, gdy mogę wykonać jedną z następujących czynności:

  • Dodaj informacje kontekstowe (takie jak stan obiektów lub co się działo)
  • Obsługuj wyjątek bezpiecznie (na przykład metoda TryX)
  • Twój system przekracza granicę usługi i dzwoni do zewnętrznej biblioteki lub interfejsu API
  • Chcesz złapać i ponownie rzucić inny typ wyjątku (być może oryginał stanowi wyjątek wewnętrzny)
  • Wyjątek został zgłoszony w ramach funkcji o niskiej wartości w tle

Jeśli to nie jeden z tych przypadków, nie dodam lokalnej try / catch. Jeśli tak, w zależności od scenariusza mogę obsłużyć wyjątek (na przykład metodę TryX, która zwraca wartość false) lub powtórzyć, aby wyjątek był obsługiwany przez globalną strategię.

Na przykład:

public bool TryConnectToDatabase()
{
  try
  {
    this.ConnectToDatabase(_databaseType); // this method will throw if it fails to connect
    return true;
  }
  catch(Exception ex)
  {
     this.Logger.Error(ex, "There was an error connecting to the database, the databaseType was {0}", _databaseType);
    return false;
  }
}

Lub przykład powtórzenia:

public IDbConnection ConnectToDatabase()
{
  try
  {
    // connect to the database and return the connection, will throw if the connection cannot be made
  }
  catch(Exception ex)
  {
     this.Logger.Error(ex, "There was an error connecting to the database, the databaseType was {0}", _databaseType);
    throw;
  }
}

Następnie wychwytujesz błąd na górze stosu i przekazujesz użytkownikowi przyjazną wiadomość.

Niezależnie od tego, jakie podejście podejmiesz, zawsze warto utworzyć testy jednostkowe dla tych scenariuszy, aby mieć pewność, że funkcjonalność się nie zmieni i nie zakłóci przepływu projektu w późniejszym terminie.

Nie wspomniałeś, w jakim języku pracujesz, ale jesteś programistą .NET i widziałeś to zbyt wiele razy, nie mówiąc już o tym.

NIE PISZ:

catch(Exception ex)
{
  throw ex;
}

Posługiwać się:

catch(Exception ex)
{
  throw;
}

Ten pierwszy resetuje ślad stosu i sprawia, że ​​złapanie najwyższego poziomu jest całkowicie bezużyteczne!

TLDR

Lokalne łapanie nie jest anty-wzorcem, często może być częścią projektu i może pomóc w dodaniu dodatkowego kontekstu do błędu.

Liath
źródło
3
Jaki jest sens rejestrowania połowu, gdy ten sam program rejestrujący byłby używany w module obsługi wyjątków najwyższego poziomu?
Euforia
Możesz mieć dodatkowe informacje (takie jak zmienne lokalne), do których nie będziesz mieć dostępu na górze stosu. Zaktualizuję przykład, aby go zilustrować.
Liath,
2
W takim przypadku wyrzuć nowy wyjątek z dodatkowymi danymi i wyjątkiem wewnętrznym.
Euforia
2
@ Euforyczny tak, też to zrobiłem, osobiście nie jestem fanem, ponieważ wymaga to stworzenia nowych rodzajów wyjątków dla prawie każdej metody / scenariusza, który moim zdaniem jest dużym obciążeniem. Dodanie tutaj wiersza dziennika (i przypuszczalnie innego na górze) pomaga zilustrować przepływ kodu podczas diagnozowania problemów
Liath
4
Java nie zmusza cię do obsługi wyjątku, zmusza cię do bycia świadomym tego. możesz albo złapać i zrobić cokolwiek, albo po prostu zadeklarować to jako coś, co funkcja może rzucić i nic z tym nie robić w funkcji .... Niewielkie naciąganie na całkiem dobrą odpowiedź!
Newtopian
4

To zależy bardzo od języka. Np. C ++ nie oferuje śledzenia stosu w komunikatach o błędach wyjątków, więc śledzenie wyjątku poprzez częste przechwytywanie logów i rzutów może być pomocne. W przeciwieństwie do tego Java i podobne języki oferują bardzo dobre ślady stosu, chociaż format tych śladów stosu może nie być bardzo konfigurowalny. Łapanie i ponowne zgłaszanie wyjątków w tych językach jest całkowicie bezcelowe, chyba że naprawdę można dodać jakiś ważny kontekst (np. Połączenie wyjątku SQL niskiego poziomu z kontekstem operacji logiki biznesowej).

Każda strategia obsługi błędów zaimplementowana poprzez odbicie jest prawie niekoniecznie mniej wydajna niż funkcjonalność wbudowana w język. Ponadto wszechstronne rejestrowanie ma nieunikniony narzut wydajności. Musisz więc zrównoważyć strumień informacji, które otrzymujesz, z innymi wymaganiami dotyczącymi tego oprogramowania. To powiedziawszy, rozwiązania takie jak PostSharp, które są zbudowane na oprzyrządowaniu na poziomie kompilatora, generalnie działają znacznie lepiej niż odbicie w czasie wykonywania.

Osobiście uważam, że rejestrowanie wszystkiego nie jest pomocne, ponieważ zawiera mnóstwo nieistotnych informacji. Dlatego jestem sceptyczny wobec zautomatyzowanych rozwiązań. Biorąc pod uwagę dobre ramy rejestrowania, może być wystarczające ustalenie wytycznych kodowania, które omawiają, jakie informacje chcesz rejestrować i jak należy je sformatować. Następnie możesz dodać rejestrowanie tam, gdzie ma to znaczenie.

Logowanie do logiki biznesowej jest znacznie ważniejsze niż logowanie do funkcji narzędziowych. A gromadzenie śladów stosu rzeczywistych raportów o awariach (które wymaga jedynie rejestrowania na najwyższym poziomie procesu) pozwala zlokalizować obszary kodu, w których rejestrowanie miałoby największą wartość.

amon
źródło
4

Kiedy widzę try/catch/logw każdej metodzie, budzi to obawy, że programiści nie mieli pojęcia, co może się wydarzyć w ich aplikacji, przyjęli najgorsze i zapobiegawczo zarejestrowali wszystko wszędzie ze względu na wszystkie błędy, których się spodziewali.

Jest to objaw, że testy jednostkowe i integracyjne są niewystarczające, a programiści są przyzwyczajeni do przechodzenia przez dużą ilość kodu w debuggerze i mają nadzieję, że w jakiś sposób dużo dzienników pozwoli im wdrożyć błędny kod w środowisku testowym i znaleźć problemy, patrząc na dzienniki.

Kod generujący wyjątki może być bardziej przydatny niż kod redundantny wychwytujący i rejestrujący wyjątki. Jeśli rzucisz wyjątek z sensownym komunikatem, gdy metoda otrzyma nieoczekiwany argument (i zarejestrujesz go na granicy usługi), jest to o wiele bardziej pomocne niż natychmiastowe zarejestrowanie zgłoszonego wyjątku jako efekt uboczny nieprawidłowego argumentu i konieczność odgadnięcia, co go spowodowało .

Wartości zerowe są przykładem. Jeśli otrzymujesz wartość jako argument lub wynik wywołania metody i nie powinna ona być pusta, wyrzuć wyjątek. Nie należy rejestrować wynikowego NullReferenceExceptionwyrzucenia pięciu linii później ze względu na wartość zerową. Tak czy inaczej otrzymujesz wyjątek, ale jeden mówi ci coś, a drugi sprawia, że ​​szukasz czegoś.

Jak powiedzieli inni, najlepiej rejestrować wyjątki na granicy usługi lub za każdym razem, gdy wyjątek nie zostanie ponownie wygenerowany, ponieważ jest obsługiwany z wdziękiem. Najważniejsza różnica polega na czymś i niczym. Jeśli Twoje wyjątki są zarejestrowane w jednym łatwo dostępnym miejscu, znajdziesz potrzebne informacje, gdy są potrzebne.

Scott Hannen
źródło
Dzięki Scott. Punkt, który podjąłeś „ Jeśli rzucisz wyjątek z sensownym komunikatem, gdy metoda otrzyma nieoczekiwany argument (i zarejestruje go na granicy usługi) ” naprawdę uderzy i pomógł mi wizualizować sytuację zawisłą wokół mnie w kontekście argumentów metody. Myślę, że warto mieć klauzulę bezpieczeństwa i wyrzucić ArgumentException w tym przypadku zamiast polegać na wyłapywaniu i rejestrowaniu szczegółów argumentów
rahulaga_dev
Scott, mam to samo uczucie. Ilekroć widzę dziennik i ponownie piszę tylko w celu zapisania kontekstu, czuję, że programiści nie mogą kontrolować niezmienników klasy lub nie mogą zabezpieczyć wywołania metody. Zamiast tego każda metoda do końca jest zawinięta w podobny try / catch / log / throw. I to jest po prostu okropne.
Maks
2

Jeśli chcesz zapisać informacje kontekstowe, które nie są jeszcze objęte wyjątkiem, umieść je w nowym wyjątku i podaj oryginalny wyjątek jako InnerException. W ten sposób zachowujesz oryginalny ślad stosu. Więc:

public static bool DoOperation(int num1, int num2)
{
    try
    {
        /* do some work with num1 and num2 */
    }
    catch (Exception ex)
    {
        throw new Exception("error occured while number 1 = {num1} and number 2 = {num2}", ex);
    }
}

Drugi parametr Exceptionkonstruktora stanowi wewnętrzny wyjątek. Następnie możesz zarejestrować wszystkie wyjątki w jednym miejscu, a pełny zapis stosu i informacje kontekstowe znajdują się w tym samym wpisie dziennika.

Możesz użyć niestandardowej klasy wyjątku, ale sprawa jest taka sama.

try / catch / log / rethrow to bałagan, ponieważ doprowadzi do mylących dzienników - np. co się stanie, jeśli w innym wątku wydarzy się inny wyjątek między rejestrowaniem informacji kontekstowych a rejestrowaniem faktycznego wyjątku w module obsługi najwyższego poziomu? try / catch / throw jest jednak w porządku, jeśli nowy wyjątek dodaje informacje do oryginału.

JacquesB
źródło
Co z oryginalnym typem wyjątku? Jeśli opakujemy, zniknie. To jest problem? Ktoś na przykład polegał na SqlTimeoutException.
Maks
@Max: Oryginalny typ wyjątku jest nadal dostępny jako wyjątek wewnętrzny.
JacquesB
To miałem na myśli! Teraz wszyscy na górze stosu wywołań, który łapał SqlException, nigdy go nie dostanie.
Maks
1

Sam wyjątek powinien zawierać wszystkie informacje niezbędne do prawidłowego rejestrowania, w tym komunikat, kod błędu i inne informacje. Dlatego nie powinno być potrzeby wychwytywania wyjątku, aby go ponownie rzucić lub rzucić inny wyjątek.

Często widzisz wzorzec kilku wyjątków wychwyconych i ponownie zgłoszonych jako powszechny wyjątek, takich jak przechwycenie DatabaseConnectionException, InvalidQueryException i InvalidSQLParameterException i ponowne zgłoszenie wyjątku DatabaseException. Chociaż do tego argumentowałbym, że wszystkie te szczególne wyjątki powinny wynikać przede wszystkim z DatabaseException, więc ponowne zgłaszanie nie jest konieczne.

Przekonasz się, że usunięcie niepotrzebnych klauzul try catch (nawet tych do czystego logowania) w rzeczywistości ułatwi zadanie, a nie będzie trudniejsze. Tylko miejsca w twoim programie, które obsługują wyjątek, powinny rejestrować wyjątek, a jeśli wszystko inne zawiedzie, program obsługi wyjątków dla całego programu dla ostatniej próby zarejestrowania wyjątku przed płynnym zamknięciem programu. Wyjątek powinien mieć ślad pełnego stosu wskazujący dokładny punkt, w którym został zgłoszony wyjątek, więc często nie jest konieczne zapewnianie rejestrowania „kontekstowego”.

To powiedziawszy, AOP może być dla Ciebie rozwiązaniem szybkiej naprawy, chociaż zwykle wiąże się to z niewielkim spowolnieniem. Zachęcam cię do całkowitego usunięcia niepotrzebnych klauzul try catch, w których nie ma wartości dodanej.

Neil
źródło
1
Sam wyjątek powinien zawierać wszystkie informacje niezbędne do prawidłowego rejestrowania, w tym komunikat, kod błędu i co nie ”. Powinny, ale w praktyce nie odwołują się do wyjątków Null, które są klasycznym przypadkiem. Na przykład nie znam żadnego języka, który powie ci zmienną, która spowodowała ją w złożonym wyrażeniu.
David Arno
1
@DavidArno Prawda, ale żaden kontekst, który mógłbyś podać, nie może być tak konkretny. W przeciwnym razie miałbyś try { tester.test(); } catch (NullPointerException e) { logger.error("Variable tester was null!"); }. Śledzenie stosu jest wystarczające w większości przypadków, ale bez tego typ błędu jest zwykle odpowiedni.
Neil