2
\$\begingroup\$

When handling errors in C#, is it acceptable/appropriate to re-throw exceptions that are caught in a try/catch?

My scenario is that I am making an HTTP request and then checking the content of the response for a specific key (access_token). If that key is missing, I throw an error.

However, as all of the code within the method is in a try/catch block (just in case HTTP requests go awry), the exception that I throw is being caught. The solution that I've settled on for now is to re-throw the error that is caught, to ensure that I don't lose additional detail that was included with the custom exception. You can see a full example of my code at the bottom of this question.

I've found a number of other possibilities though, and I'd be interested to know what the C# community thinks is the best way to handle such scenarios (even if it turns out that none of them are appropriate!). There are of course other questions/resources that are similar to this, but some of the information is ambiguous and being a novice at C#, I'd like get a bit of extra context with a real-life example.

Possibilities

  1. A separate catch block to re-throw the known error (see example 1).
  2. Check for the type of exception in a single catch block (see example 2).
  3. Return an exception should 'access_token' not be present (as opposed to throwing it).
  4. Don't wrap all the code in a try\catch.

Example 1

catch (WhException ex)
{
    // Capture and rethrow a 'WhException', ensurig that the detail is not lost.
    throw ex;
}
catch (Exception ex)
{
    // Capture any generic exception and retrhow it as a 'WhException'.
    throw new WhException("An error occurred whilst attempting to generate a bearer access token.", ex.Message, ex);
}

Example 2

catch (Exception ex)
{
    if (ex is WhException)
        // Capture and rethrow a 'WhException', ensurig that the detail is not lost.
        throw ex;
    else
        // Capture any generic exception and retrhow it as a 'WhException'.
        throw new WhException("An error occurred whilst attempting to generate a bearer access token.", ex.Message, ex);
}

Full working example

public static async Task<string> GetAzureAccessTokenAsync(this HttpClient http, WhConfigRaw config)
{
    try
    {
        Dictionary<string, string> form = new Dictionary<string, string>()
        {
            { "grant_type", "client_credentials" },
            { "client_id", config.ServicePrincipalId },
            { "client_secret", config.ServicePrincipalKey },
            { "resource", AppSettings.Get("AzureCoreUri") }
        };
        FormUrlEncodedContent requestContent = new FormUrlEncodedContent(form);
        string uri = string.Format(AppSettings.Get("ClientCredentialsUri"), config.TenantId);

        // Query the Azure management API to get client credentials.
        HttpResponseMessage response = await http.PostAsync(uri, requestContent);

        // Read the content of the response for the Azure management API.
        JObject content = await response.Content.ReadAsJsonAsync<JObject>();

        // Check whether or not the content of the response contains and 'access_token'.
        if (!content.ContainsKey("access_token"))
        {
            throw new WhException("An error occurred whilst attempting to generate a bearer access token.", content);
        }

        return content["access_token"].ToString();
    }
    catch (WhException ex)
    {
        // Capture and rethrow a 'WhException', ensurig that the detail is not lost.
        throw ex;
    }
    catch (Exception ex)
    {
        // Capture any generic exception and retrhow it as a 'WhException'.
        throw new WhException("An error occurred whilst attempting to generate a bearer access token.", ex.Message, ex);
    }
}
\$\endgroup\$

closed as primarily opinion-based by tinstaafl, pacmaninbw, VisualMelon, yuri, t3chb0t Jun 14 at 17:28

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Do you do anything with the exception besides catch it? If not, then there's no point catching it in the first place. \$\endgroup\$ – Chris Dunaway Jun 14 at 16:48
  • \$\begingroup\$ Thanks for your comment. Exceptions are bubbled up so that they may be handled gracefully. The bottom catch (Exception ex) would only really be for informational purposes, as an error there would likely mean a configuration error. But the other exception contains error messages which must be presented to the users, hence my uncertainty as to whether or not I'm handling this correctly. \$\endgroup\$ – David Gard Jun 17 at 15:31
3
\$\begingroup\$

Deciding which exceptions to handle, rethrow, wrap in other exceptions is a design decision. Some exceptions might be caught to branch to a different flow. Naively catching all exceptions is a bad call. Guidelines for handling and throwing exceptions


Some exceptions should be propagated all the way up, because of the integrity of the application or even your machine cannot be preserved. Be careful when encountering OutOfMemoryException, BadImageFormatException, StackOverflowException.

Would you really want to wrap/hide the aforementioned exceptions in a custom exception?

catch (Exception ex)
{
    // Capture any generic exception and retrhow it as a 'WhException'.
    throw new WhException("An error occurred whilst ..", ex.Message, ex);
}

Keep in mind that detail is lost when you catch and throw ex;. A new stacktrace is created.

catch (WhException ex)
{
    // Capture and rethrow a 'WhException', ensurig that the detail is not lost.
    throw ex;
}

Use throw; to preserve stacktrace.

catch (WhException ex)
{
    throw; // preserves stacktrace
}

If you want to rethrow outside the catch block you could use ExceptionDispatchInfo. Suppose Exception error is declared before the try:

catch (WhException ex)
{
    error = ExceptionDispatchInfo.Capture(ex);
}

error?.Throw(); // preserves stacktrace

Consider throwing these well-known exceptions for common situations:

  • ArgumentNullException : when a mandatory argument is not provided
  • ArgumentException: when an argument does not meet the preconditions to run
  • AggregateException: when packing multiple exceptions together

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for your response. I didn't know about the lose of the stack trace when using throw ex, so that's def something I'll change. \$\endgroup\$ – David Gard Jun 17 at 15:32

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