Czy nadal jest to antypattern, jeśli zarejestrujemy komunikat wyjątku i wyrzucimy inny wyjątek?

18

Nasza aplikacja internetowa używa ExceptionMapperdo mapowania niektórych wyjątków Response. Komunikaty o wyjątkach rejestrujemy przed zgłoszeniem nowego wyjątku w następujący sposób:

catch (SomeException ex) {
  LOG.error(ex.getMessage());
  throw new MyException(ex.getMessage());
}

Nie rzucamy ponownie tego samego wyjątku , więc moje pytanie brzmi, czy byłoby to uważane za antypater . A zatem, czy lepiej byłoby usunąć rejestrowanie w podobnych miejscach i przenieść je do kilku ExceptionMapperklas w następujący sposób:

@Provider
public class MyExceptionMapper implements ExceptionMapper<MyException> {

  // bla bla 

  @Override
  public Response toResponse(final MyException ex) {
    LOG.error(ex.getMessage());
    return Response.status(400).entity("something").build();
  }
}
Diyarbakir
źródło
3
Zatrzymałbym cię, jak tylko się zalogujesz ex.getMessage(), to już źle.
biziclop
Można argumentować, że zgłoszenie tekstu wyjątku jest zawsze złym pomysłem .
IMO nie jest w ogóle uczciwe, aby w ogóle nie wspominać, że jest to usługa internetowa. To naprawdę zmienia zasady gry, ponieważ na przykład użycie zwykłej mechaniki obsługi wyjątków może stanowić zagrożenie bezpieczeństwa; nie chcesz, aby jakikolwiek wyjątek był wysyłany z powrotem w odpowiedzi na błąd 500, który należy sprawdzić i przefiltrować. Agresywne rejestrowanie na miejscu jest również znacznie częstsze w przypadku systemów, które bezpośrednio go wywołują. Rejestrowanie śledzenia stosu w wywołaniach serwisowych, które są wywoływane wielokrotnie, może prowadzić do niemożliwych do zarządzania rozmiarów plików dziennika i problemów z wydajnością.
@Gimby W tym przypadku OP nie wysyła żadnych szczegółów błędu w odpowiedzi (prawdopodobnie treść „wystąpił błąd” na płycie głównej). Jak zdiagnozować problem bez śledzenia stosu? Rolowane rejestratory rutynowo dbają o rozmiar pamięci, a dane dziennika są zawstydzająco kompresowalne.
Marko Topolnik,

Odpowiedzi:

36

W rzeczywistości kod zawiera nie tylko jeden, ale trzy anty-wzory:

  1. zaloguj się i wrzuć;
  2. wrzuć bez zawijania oryginalnej przyczyny;
  3. zaloguj tylko komunikat, a nie stos śledzenia (to jest najgorszy).

Jeśli zastosujesz się do najlepszych praktyk, aby:

  1. w ogóle nie wychwytywać (niech wyjątek rozprzestrzeni się sam);
  2. jeśli zostanie zmuszony do złapania zaznaczonego wyjątku, zawiń go do niezaznaczonego i powtórz;
  3. nigdy nie loguj niczego poza najwyższym poziomem;
  4. zaloguj cały wyjątek stacktrace za pomocą log.error("Error occurred", e);

wtedy nie napotkalibyście żadnych dylematów, w tym także obecnego, ponieważ zalogowany ślad stosu obejmowałby również wszystkie opakowane wyjątki.

Marko Topolnik
źródło