2

Exogenous exceptions are that kind of exception that are unavoidable because they are throwed outside of our system's pristine logic.

A good practice is to throw wrapped exceptions in lower layers like the DAL.

However, in my application i have a lot of filesystem access, and I came up with recursive functions, like this :

public void Delete()
{
    try
    {
        _file.Delete();
    }
    catch (Exception e) when (e.FileSystem()) 
    {
        new Presentation.FSErrorDialog(e, _file, FSVerb.Delete).ShowDialog(Delete);
    }
}

Where

  • FileSystem()

      public static bool FileSystem(this Exception e)
          => e is IOException or UnauthorizedAccessException or NotSupportedException or  System.Security.SecurityException;
    
  • FSErrorDialog is a subclass of TaskDialogPage

  • ShowDialog() takes an Action delegate that is executed when the Retry button is clicked

As you can see, DeleteFile() doesn't throw any exceptions. It simplies recurses and warns the user. Here's how the dialog would look :

FSErrorDialog TaskDialogPage

However, to respect the principle of multi-layer architecture that layers shouldn't call upper layers, it should throw an exception and let the presentation layer catch it.

But doing this is so convenient ! Is it acceptable ?

5
  • 4
    "A good practice is to throw wrapped exceptions in lower layers like the DAL" -- I disagree because this is such a general/blanket statement and in many situations there is simply no point. Quite often the only thing you can usefully do is catch(Exception) then take some generic action like write to a log and retry or show the user a generic "Something went wrong" message. Wrapping exceptions is only useful if it adds something you actually use such as contextual information which improves debugging or error handling, otherwise it can just be unnecessary noise. Nov 26, 2021 at 22:03
  • what about OutOfMemoryException ?
    – Scover
    Nov 27, 2021 at 9:59
  • 1
    I can't think of a reason to wrap OutOfMemoryException; firstly on the basis that it seems unlikely there'd be any useful information to add (it probably needs a crash dump to analyse properly), secondly because attempting to wrap and add extra information could fail if the system is out of memory. The exception seems to already have enough information to take action; e.g. write the full exception to a log file then possibly just let the program crash. Out of memory may affect the host device, so the device owner might appreciate a "nasty" error they can see and report to a developer. Nov 27, 2021 at 11:18
  • I mean, catch(Exception) is bad practice because it catches exceptions like DivideByZeroExceptions and other exceptions that should not be catched because they are the consequence of a bug in the code. OutOfMemoryException should never be catched
    – Scover
    Nov 27, 2021 at 11:40
  • 2
    I disagree. Having a generic catch (Exception) handler at the top level of your program to handle those uncaught exceptions is not at all bad practice -- the program needs an opportunity to log those somewhere for a developer to analyse faults when they're reported, otherwise the program will just crash and that information would otherwise just be lost. The fact they are a consequence of a bug in the code is exactly why they really need to be caught. Nov 27, 2021 at 11:47

3 Answers 3

6

A good practice is to throw wrapped exceptions in lower layers like the DAL.

It is a good practice to throw exceptions from lower layers to report problems. They don't have to be wrapped exceptions, unless the wrapping adds useful information to the upper layers for handling an exception.

Another reason for wrapping could also be to isolate the upper layers from exception classes that are specific to a particular third-party dependency and there is a chance that the dependency will be swapped out for another dependency (with different exception classes).

Is it acceptable ?

Unless your recursive function is part of the Presentation layer, no your function is not acceptable.

The function is not acceptable, because it breaks the layer boundaries and contains presentation logic (I need to show a dialog) outside of the Presentation layer.

However, if you move this function as-is to the Presentation layer, then it would become acceptable.

3

Attribution

This answer is to augment Bart van Ingen Schenau's answer with code example.

Preface

As always, the question of ...

Handling exogenous exceptions in layered architecure

... is only a smell. A smell is indicative of something bigger; but it is never the only problem we're concerned with.

The quick answer to that question is that "it depends". In particular, for languages and layers that already have well-designed exception systems (such as the case of C# ecosystem), the cost of wrapping outweighs the benefits from doing so.

Some of my subjective takes

It may be the case that deleting a file is considered such as mundane operation that some people do not consider it to be worthy of being "business logic".

It may also be the case that, considering retry cannot happen without a human commanding it, that some people consider retry to be a purely presentation-layer concern.

In other words, some people might not consider your code example to be worthy of layer separation. The task of deleting a file may be "too simple" to justify that effort.

Regarding the layers

In GUI applications, business logic and presentation layer is very often intertwined tightly. For this reason, GUI applications don't start out with a separation of business and presentation layers, except for opportunistic or overriding reasons. (See below.)

Tendency to become intertwined

This call stack illustrates why it is necessarily so:

  • User initiates a multi-step task (on Presentation Layer)
    • Business logic for a multi-step task
      • Business logic for a single-step task
        • Exceptional case where user must be asked what to do
          • Displays dialog (back to GUI; though not necessarily being a "Presentation Layer")

Can't I simply back out from the entire call stack? (e.g. by throwing an exception)

If it is possibly to do so, yes, definitely do it. Code is much simpler if this is possible.

The possibility to do so is called transactionality: If there's a problem half way through, the entire multi-step operation can be aborted, without any harm to system consistency.

This is not always possible; it depends on the business requirements and application design.

Opportunistic reasons for layer separation (if it happens to be doable, let's do it)

  • Certain business logic happens to be easy to isolate and decouple from the presentation layer, and easy to be isolated into its own C# assembly.
    • This is facilitated by C# delegates; se my code example below.
    • This is not to say they're independent; they are dependent at runtime, but only after the delegates have been injected.
    • The decoupling aids in C# code compilation and packaging, and not much else.

Business requirement reason for layer separation (something that overrides all software architectural concerns)

  • High-level requirements make it necessary to abstract out the Presentation Layer, which requires a strict separation of layers, at the cost of much higher code complexity.
  • Example:
    • Code needs to run on multiple platforms, e.g. Desktop and Mobile
    • There needs to be one C# assembly for business logic, and a number of platform-specific C# assemblies just for the presentation layer.

Solving the dilemma

We can help decouple the layers by delegation (Func, Action, etc). In particular:

  • Presentation Layer can implement various functions (callbacks) for simple things such as showing a dialog.
  • Business Layer functions can be customized by accepting delegates from the caller, allowing the caller to decide what to do

What to do if delegation (callbacks) aren't sufficient to solve the dilemma

If callbacks aren't enough, try creating interfaces.

However, given a sufficiently complicated multi-step task, the application architecture may eventually evolve toward an interactive business rule engine, or "wizard". Think of it as an interpreter for a business scripting language.


Code example

Your code example is potentially unbounded recursive (imagine the user keeps clicking retry despite repeated failures), but it's trivial to rewrite it to make it bounded and without reentrancy, by converting reentrancy into conditional repetition (while-loop).


// Business Layer code

public void DeleteWithRetry(
    Func<Exception, FileSystemInfo, FSVerb, bool> checkRetryFunc)
{
    // Optional feature:
    // To prevent the user from clicking retry endlessly,
    // one can implement a fixed retry limit to stop the user
    // at some point. This can be implemented with a for-loop.

    while (true)
    {
        try
        {
            _file.Delete();
            break; // or return; to exit the while loop.
        }
        catch (Exception e) when (e.FileSystem()) 
        {
            bool shouldRetry = checkRetryFunc?.(e, _file, FSVerb.Delete) ?? false;
            if (!shouldRetry) {
                throw;
            }
        }
        // automatically retries unless it breaks from while.
    }
}

// Presentation Layer code

bool AskFileSysOpRetry(Exception e, FileSystemInfo entry, FSVerb verb)
{
    bool shouldRetry = false;
    var dialog = new Presentation.FSErrorDialog(e, entry, verb);
    dialog.ShowDialog(() => { shouldRetry = true; });
    return shouldRetry;
}

// Code that needs to invoke a business layer function, a.k.a. to delete a file
// **together with** interactivity (presentation layer)

public void SomeBiggerTask()
{
    DeleteWithRetry(AskFileSysOpRetry);
    DoOtherStuffWithRetry(AskOtherStuffRetry);
}
3

A secure application should never leak a DAL exception to the user interface. The exception might contain sensitive information (e.g. we now know something about your application server's folder structure, and possibly the name of the user account it runs under) or contain malicious information (e.g. if the exception came from a third party who has been compromised). The retry mechanism itself exposes you to a time-of-check time-of-use attack.

Also, it's unkind to your users. You are expecting them to figure out how to fix a problem that you didn't even see coming. You're supposed to be the expert. They might not even speak the same language.

If you want some sort of fault tolerance for low level failures, figure out what they could be and define alternative flows for them (and write unit tests for them). If you want to be fault tolerant of unexpected failures, display a generic message and allow the user to retry the entire business operation, not the low level operation. If you need diagnostic information, emit it to a secure event log, and make it available to a customer support tool if you need to.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.