2
\$\begingroup\$

This is a fire and forget task from within an existing WebAPI service. The code below simply sends a log event (might be that a client has connected to the service to perform some task) to another service and if it fails should not return an error to the client.

I simply want the exception logged and forgotten about, meanwhile the service will continue to carrying out the original task.

I've used ContinueWith and am wondering if this is the correct way to do it.

Task.Factory.StartNew(() =>
{
    var client = (HttpWebRequest) WebRequest.Create(_url);

    client.Method = "POST";
    client.ContentType = "application/json";

    var bytes = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(log));

    var requestStream = client.GetRequestStream();

    requestStream.Write(bytes, 0, bytes.Length);

    requestStream.Close();

    var response = client.GetResponse();

    requestStream.Close();
    response.Close();

}).ContinueWith((ca) =>
{
    if (ca.Exception != null)
    {
        _logger.LogException(LogLevel.Error, ca.Exception.Message, ca.Exception);
    }

}, TaskContinuationOptions.OnlyOnFaulted);
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review! As a courtesy to other users, please declare your cross-post. \$\endgroup\$ Commented Apr 29, 2016 at 5:00

1 Answer 1

2
\$\begingroup\$

ContinueWith() using TaskContinuationOptions.OnlyOnFaulted is right way to do it, especially when you do not have control over first task code (for example because it comes from an unknown method as Task).

However in your example you do not need a continuation task to handle errors, you may (and should) enclose your code inside a try/catch block and handle exceptions in-place:

Task.Factory.StartNew(() =>
{
    try
    {
        var client = (HttpWebRequest)WebRequest.Create(_url);

        // ...
    }
    catch (Exception e)
    {
         _logger.LogException(LogLevel.Error, e.Message, e);
    }
});

Note that this is just first step. First of all you're catching a generic Exception, if it's just for logging purposes it may be fine but you should consider if you have better options (error recovering? retrying?) handling more specific exceptions. For example you may want to handle WebException and check if it's a timeout, there may be a network connection issue and you may want to wait and retry like in System.Net.WebException: The remote name could not be resolved.

Now you should correctly dispose resources you're using. If an error occurs, for example, after you acquired request stream but before you close it then connection will be immediately released. To simplify this you have a nice C# statement, using:

using (var requestStream = client.GetRequestStream())
{
    requestStream.Write(bytes, 0, bytes.Length);
    // ...
}

Also note that you should close request stream before asking for response stream, even if it's not an issue it's easier to do it right if you have using in-place.

\$\endgroup\$

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.