Prova / Catch / Log / Rethrow - Anti Pattern?

Il problema non è il blocco catch locale, il problema è il log e rethrow . O gestire l'eccezione o avvolgerla con una nuova eccezione che aggiunge contesto aggiuntivo e lancia quello. Altrimenti verrai eseguito in più voci di registro duplicate per la stessa eccezione.

L'idea qui è di migliorare la capacità di eseguire il debug dell'applicazione.

Esempio 1: gestirlo

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

Se gestisci l'eccezione, puoi facilmente ridimensionare l'importanza della voce del registro delle eccezioni e non c'è motivo di trascinare quell'eccezione nella catena. È già stato risolto.

La gestione di un'eccezione può includere informare gli utenti che si è verificato un problema, registrare l'evento o semplicemente ignorarlo.

NOTA: se ignori intenzionalmente un'eccezione, ti consiglio di fornire un commento nella clausola catch vuota che indichi chiaramente perché. Ciò consente ai futuri manutentori di sapere che non è stato un errore o una programmazione pigra. Esempio:

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)
}

Esempio n. 2: aggiungi contesto aggiuntivo e lancia

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

L'aggiunta di un contesto aggiuntivo (come il numero di riga e il nome del file nel codice di analisi) può aiutare a migliorare la capacità di eseguire il debug dei file di input, presupponendo che il problema sia presente. Questo è un tipo di caso speciale, quindi riavvolgere un'eccezione in "ApplicationException" solo per rebrandarlo non aiuta il debug. Assicurati di aggiungere ulteriori informazioni.

Esempio n. 3: non fare nulla con l'eccezione

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

In questo caso finale, lasci semplicemente che l'eccezione se ne vada senza toccarlo. Il gestore di eccezioni nel livello più esterno può gestire la registrazione. La clausola finally viene utilizzata per assicurarsi che tutte le risorse necessarie per il tuo metodo vengano eliminate, ma non è questo il luogo in cui registrare che è stata generata l'eccezione.

Non credo che le catture locali siano un anti-pattern, infatti se ricordo bene è effettivamente applicato in Java!

Ciò che per me è fondamentale quando si implementa la gestione degli errori è la strategia generale. Potresti desiderare un filtro che raccolga tutte le eccezioni al limite del servizio, potresti voler intercettarlo manualmente: entrambi vanno bene fintanto che esiste una strategia generale, che rientrerà negli standard di codifica dei tuoi team.

Personalmente mi piace catturare errori in una funzione quando posso fare una delle seguenti azioni:

• Aggiungi informazioni contestuali (come lo stato degli oggetti o cosa stava succedendo)
• Gestisci l'eccezione in modo sicuro (come un metodo TryX)
• Il tuo sistema sta attraversando un limite di servizio e chiama in una libreria o API esterna
• Vuoi catturare e rilanciare un diverso tipo di eccezione (forse con l'originale come eccezione interna)
• L'eccezione è stata generata come parte di alcune funzionalità di background di basso valore

Se non è uno di questi casi, non aggiungo un try / catch locale. Se lo è, a seconda dello scenario, posso gestire l'eccezione (ad esempio un metodo TryX che restituisce un falso) o ripensare in modo che l'eccezione venga gestita dalla strategia globale.

Ad esempio:

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;
  }
}

O un esempio di rethrow:

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;
  }
}

Quindi rilevi l'errore in cima allo stack e presenta all'utente un messaggio di facile utilizzo.

Indipendentemente dall'approccio che prendi, vale sempre la pena di creare test unitari per questi scenari, in modo da assicurarti che la funzionalità non cambi e interrompa il flusso del progetto in un secondo momento.

Non hai menzionato la lingua in cui lavori ma sei uno sviluppatore .NET e lo hai visto troppe volte per non menzionarlo.

NON SCRIVERE:

catch(Exception ex)
{
  throw ex;
}

Usa:

catch(Exception ex)
{
  throw;
}

Il primo reimposta la traccia dello stack e rende assolutamente inutile il tuo livello più alto!

TLDR

L'acquisizione locale non è un anti-pattern, può spesso far parte di un design e può aiutare ad aggiungere ulteriore contesto all'errore.

Dipende molto dalla lingua. Per esempio. C ++ non offre tracce di stack nei messaggi di errore delle eccezioni, quindi può essere utile tracciare l'eccezione tramite frequenti catch-log-rethrow. Al contrario, Java e lingue simili offrono ottime tracce di stack, sebbene il formato di queste tracce dello stack potrebbe non essere molto configurabile. Le eccezioni di catching e rethrowing in queste lingue sono assolutamente inutili a meno che non si possa davvero aggiungere un contesto importante (ad esempio collegando un'eccezione SQL di basso livello al contesto di un'operazione di business logic).

Qualsiasi strategia di gestione degli errori implementata tramite la reflection è quasi necessariamente meno efficiente della funzionalità incorporata nella lingua. Inoltre, la registrazione pervasiva ha un overhead di prestazioni inevitabile. Quindi è necessario bilanciare il flusso di informazioni che si ottiene rispetto ad altri requisiti per questo software. Detto questo, soluzioni come PostSharp che sono costruite su strumentazione a livello di compilatore di solito sono molto meglio della riflessione in fase di esecuzione.

Personalmente ritengo che la registrazione di tutto non sia utile, perché include un sacco di informazioni irrilevanti. Sono quindi scettico sulle soluzioni automatizzate. Dato un buon quadro di registrazione, potrebbe essere sufficiente avere una linea guida di codifica concordata che discuta quale tipo di informazioni si desidera registrare e come queste informazioni dovrebbero essere formattate. È quindi possibile aggiungere la registrazione dove è importante.

L'accesso alla logica aziendale è molto più importante dell'accesso alle funzioni di utilità. E la raccolta di tracce dello stack di report sugli arresti anomali nel mondo reale (che richiede solo la registrazione al livello più alto di un processo) consente di individuare le aree del codice in cui la registrazione avrebbe il maggior valore.

Quando vedo try/catch/log in ogni metodo solleva preoccupazioni sul fatto che gli sviluppatori non avessero idea di cosa potrebbe o non potrebbe accadere nella loro applicazione, presumibilmente il peggiore, e registrò preventivamente tutto dappertutto a causa di tutti i bug che si aspettavano.

Questo è un sintomo che i test di unità e integrazione sono insufficienti e gli sviluppatori sono abituati a passare un sacco di codice nel debugger e sperano che in qualche modo un sacco di registrazione consentirà loro di distribuire codice bug in un ambiente di test e di trovare i problemi guardando i registri.

Codice che le eccezioni lancia possono essere più utili del codice ridondante che cattura e registra le eccezioni. Se si lancia un'eccezione con un messaggio significativo quando un metodo riceve un argomento inatteso (e lo registra al limite del servizio) è molto più utile di registrare immediatamente l'eccezione generata come effetto collaterale dell'argomento non valido e dover indovinare cosa lo ha causato .

I nulli sono un esempio. Se ottieni un valore come argomento o il risultato di una chiamata al metodo e non dovrebbe essere nullo, lancia allora l'eccezione. Non registrare solo il% co_de risultante generato cinque righe successive a causa del valore null. In entrambi i casi ottieni un'eccezione, ma uno ti dice qualcosa mentre l'altro ti fa cercare qualcosa.

Come detto da altri, è meglio registrare le eccezioni al limite del servizio, o ogni volta che un'eccezione non viene retrocessa perché viene gestita correttamente. La differenza più importante è tra qualcosa e niente. Se le tue eccezioni sono registrate in un posto che è facile da raggiungere, troverai le informazioni che ti servono quando ti servono.

L'eccezione stessa dovrebbe fornire tutte le informazioni necessarie per una corretta registrazione, inclusi messaggio, codice di errore e cosa no. Pertanto non dovrebbe essere necessario rilevare un'eccezione solo per ripensarla o generare un'eccezione diversa.

Spesso viene visualizzato lo schema di diverse eccezioni rilevate e riconfigurate come eccezione comune, ad esempio l'acquisizione di un'eccezione DatabaseConnectionException, InvalidQueryException e InvalidSQLParameterException e il risanamento di un'eccezione DatabaseException. Anche se a questo, direi che tutte queste eccezioni specifiche dovrebbero derivare da DatabaseException in primo luogo, quindi non è necessario ricombinarlo.

Scoprirai che rimuovere le clausole di catch try non necessarie (anche quelle per scopi puramente logistici) renderà il lavoro più facile, non più difficile. Solo i punti del programma che gestiscono l'eccezione devono registrare l'eccezione e, in caso contrario, un gestore di eccezioni a livello di programma per un ultimo tentativo di registrazione dell'eccezione prima di uscire con garbo dal programma. L'eccezione dovrebbe avere una traccia dello stack completo che indica il punto esatto in cui è stata generata l'eccezione, quindi spesso non è necessario fornire la registrazione "contestuale".

Questo ha detto che AOP potrebbe essere una soluzione rapida per te, sebbene di solito comporti un leggero rallentamento complessivo. Ti incoraggio a rimuovere invece le clausole di catch try non necessarie interamente dove non viene aggiunto nulla di valore.

Se è necessario registrare le informazioni di contesto che non sono già nell'eccezione, lo si avvolge in una nuova eccezione e fornire l'eccezione originale come InnerException . In questo modo hai ancora la traccia dello stack originale preservata. Quindi:

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);
    }
}

Il secondo parametro del costruttore Exception fornisce un'eccezione interna. Quindi è possibile registrare tutte le eccezioni in un unico punto, e si ottiene comunque l'intera traccia dello stack e le informazioni contestuali nella stessa voce del registro.

Potresti voler utilizzare una classe di eccezioni personalizzata, ma il punto è lo stesso.

try / catch / log / rethrow è un pasticcio perché porterà a log confusi - ad es. cosa succede se si verifica un'eccezione diversa in un altro thread tra la registrazione delle informazioni contestuali e la registrazione dell'eccezione effettiva nel gestore di livello superiore? try / catch / throw va bene però, se la nuova eccezione aggiunge informazioni all'originale.