Skip to content

Fix to issue in #466 and others. ExecuteTaskAsync should not throw exceptions #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 22, 2015
Merged

Fix to issue in #466 and others. ExecuteTaskAsync should not throw exceptions #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 14 additions & 36 deletions RestSharp.IntegrationTests/AsyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,30 +87,16 @@ public void Can_Handle_Exception_Thrown_By_OnBeforeDeserialization_Handler()
var client = new RestClient(baseUrl);
var request = new RestRequest("success");

request.OnBeforeDeserialization += response =>
request.OnBeforeDeserialization += r =>
{
throw new Exception(ExceptionMessage);
};

var task = client.ExecuteTaskAsync<Response>(request);

try
{
// In the broken version of the code, an exception thrown in OnBeforeDeserialization causes the task to
// never complete. In order to test that condition, we'll wait for 5 seconds for the task to complete.
// Since we're connecting to a local server, if the task hasn't completed in 5 seconds, it's safe to assume
// that it will never complete.
Assert.True(task.Wait(TimeSpan.FromSeconds(5)),
"It looks like the async task is stuck and is never going to complete.");
}
catch (AggregateException e)
{
Assert.Equal(1, e.InnerExceptions.Count);
Assert.Equal(ExceptionMessage, e.InnerExceptions.First().Message);
return;
}

Assert.True(false, "The exception thrown from OnBeforeDeserialization should have bubbled up.");
task.Wait();
var response = task.Result;
Assert.Equal(ExceptionMessage, response.ErrorMessage);
Assert.Equal(ResponseStatus.Error, response.ResponseStatus);
}
}

Expand Down Expand Up @@ -233,15 +219,12 @@ public void Can_Timeout_GET_TaskAsync()
//Half the value of ResponseHandler.Timeout
request.Timeout = 500;

AggregateException agg = Assert.Throws<AggregateException>(
delegate
{
var task = client.ExecuteTaskAsync(request);
task.Wait();
});

Assert.IsType(typeof(WebException), agg.InnerException);
Assert.Equal("The request timed-out.", agg.InnerException.Message);
var task = client.ExecuteTaskAsync(request);
task.Wait();
var response = task.Result;
Assert.Equal(ResponseStatus.TimedOut, response.ResponseStatus);

}
}

Expand All @@ -258,15 +241,10 @@ public void Can_Timeout_PUT_TaskAsync()
//Half the value of ResponseHandler.Timeout
request.Timeout = 500;

AggregateException agg = Assert.Throws<AggregateException>(
delegate
{
var task = client.ExecuteTaskAsync(request);
task.Wait();
});

Assert.IsType(typeof(WebException), agg.InnerException);
Assert.Equal("The request timed-out.", agg.InnerException.Message);
var task = client.ExecuteTaskAsync(request);
task.Wait();
var response = task.Result;
Assert.Equal(ResponseStatus.TimedOut, response.ResponseStatus);
}
}

Expand Down
54 changes: 32 additions & 22 deletions RestSharp.IntegrationTests/NonProtocolExceptionHandlingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,14 @@ public void Task_Handles_Non_Existent_Domain()
Method = Method.GET
};

AggregateException agg = Assert.Throws<AggregateException>(
delegate
{
var response = client.ExecuteTaskAsync<StupidClass>(request);
var task = client.ExecuteTaskAsync<StupidClass>(request);
task.Wait();

response.Wait();
});
var response = task.Result;

Assert.IsType(typeof(WebException), agg.InnerException);
Assert.Equal("Unable to connect to the remote server", agg.InnerException.Message);

//var client = new RestClient("http://nonexistantdomainimguessing.org");
//var request = new RestRequest("foo");
//var response = client.ExecuteTaskAsync(request);

//Assert.Equal(ResponseStatus.Error, response.Result.ResponseStatus);
Assert.IsType(typeof(WebException), response.ErrorException);
Assert.Equal("Unable to connect to the remote server", response.ErrorException.Message);
Assert.Equal(ResponseStatus.Error, response.ResponseStatus);
}

/// <summary>
Expand All @@ -73,14 +65,11 @@ public void Handles_Server_Timeout_Error()

Assert.NotNull(response.ErrorException);
Assert.IsAssignableFrom(typeof(WebException), response.ErrorException);
Assert.Equal("The operation has timed out", response.ErrorException.Message);
Assert.Contains("The operation has timed out", response.ErrorException.Message);
}
}

[Fact]
// The asserts get trapped by a catch block and then added to the response.
// Then the second assert is hit and it just hangs indefinitely.
// Not sure why it can't break out.
public void Handles_Server_Timeout_Error_Async()
{
const string baseUrl = "http://localhost:8888/";
Expand All @@ -102,10 +91,31 @@ public void Handles_Server_Timeout_Error_Async()

Assert.NotNull(response);
Assert.Equal(response.ResponseStatus, ResponseStatus.TimedOut);
Assert.NotNull(response.ErrorException);
Assert.IsAssignableFrom(typeof(WebException), response.ErrorException);
Assert.Equal(response.ErrorException.Message, "The request timed-out.");
}
}

[Fact]
public void Handles_Server_Timeout_Error_AsyncTask()
{
const string baseUrl = "http://localhost:8888/";

//Assert.NotNull(response.ErrorException);
//Assert.IsAssignableFrom(typeof(WebException), response.ErrorException);
//Assert.Equal(response.ErrorException.Message, "The operation has timed out");
using (SimpleServer.Create(baseUrl, TimeoutHandler))
{
var client = new RestClient(baseUrl);
var request = new RestRequest("404") { Timeout = 500 };

var task = client.ExecuteTaskAsync(request);
task.Wait();
IRestResponse response = task.Result;
Assert.NotNull(response);
Assert.Equal(response.ResponseStatus, ResponseStatus.TimedOut);

Assert.NotNull(response.ErrorException);
Assert.IsAssignableFrom(typeof(WebException), response.ErrorException);
Assert.Equal(response.ErrorException.Message, "The request timed-out.");
}
}

Expand All @@ -128,7 +138,7 @@ public void Handles_Server_Timeout_Error_With_Deserializer()
Assert.Null(response.Data);
Assert.NotNull(response.ErrorException);
Assert.IsAssignableFrom(typeof(WebException), response.ErrorException);
Assert.Equal("The operation has timed out", response.ErrorException.Message);
Assert.Contains("The operation has timed out", response.ErrorException.Message);
}
}

Expand Down
10 changes: 10 additions & 0 deletions RestSharp/Http.Async.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,19 @@ private void ResponseCallback(IAsyncResult result, Action<HttpResponse> callback

private static void ExecuteCallback(HttpResponse response, Action<HttpResponse> callback)
{
PopulateErrorForIncompleteResponse(response);
callback(response);
}

private static void PopulateErrorForIncompleteResponse(HttpResponse response)
{
if (response.ResponseStatus != ResponseStatus.Completed && response.ErrorException == null)
{
response.ErrorException = response.ResponseStatus.ToWebException();
response.ErrorMessage = response.ErrorException.Message;
}
}

partial void AddAsyncHeaderActions()
{
#if SILVERLIGHT
Expand Down
18 changes: 2 additions & 16 deletions RestSharp/RestClient.Async.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,14 +275,7 @@ public virtual Task<IRestResponse<T>> ExecuteTaskAsync<T>(IRestRequest request,
{
taskCompletionSource.TrySetCanceled();
}
else if (response.ErrorException != null)
{
taskCompletionSource.TrySetException(response.ErrorException);
}
else if (response.ResponseStatus != ResponseStatus.Completed)
{
taskCompletionSource.TrySetException(response.ResponseStatus.ToWebException());
}
//Don't run TrySetException, since we should set Error properties and swallow exceptions to be consistent with sync methods
else
{
taskCompletionSource.TrySetResult(response);
Expand Down Expand Up @@ -384,14 +377,7 @@ public virtual Task<IRestResponse> ExecuteTaskAsync(IRestRequest request, Cancel
{
taskCompletionSource.TrySetCanceled();
}
else if (response.ErrorException != null)
{
taskCompletionSource.TrySetException(response.ErrorException);
}
else if (response.ResponseStatus != ResponseStatus.Completed)
{
taskCompletionSource.TrySetException(response.ResponseStatus.ToWebException());
}
//Don't run TrySetException, since we should set Error properties and swallow exceptions to be consistent with sync methods
else
{
taskCompletionSource.TrySetResult(response);
Expand Down