Skip to content

Commit

Permalink
Modify TemplateRenderer to use ThrowIfFaulted instead of Task.Wait
Browse files Browse the repository at this point in the history
  • Loading branch information
pranavkm committed Aug 21, 2014
1 parent 76ecb73 commit 9c4df46
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 1 deletion.
25 changes: 25 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/Internal/TaskHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Threading.Tasks;

namespace Microsoft.AspNet.Mvc.Internal
{
/// <summary>
/// Utility methods for dealing with <see cref="Task"/>.
/// </summary>
public static class TaskHelper
{
/// <summary>
/// Throws the first faulting exception for a task which is faulted. It preserves the original stack trace when
/// throwing the exception.
/// </summary>
/// <remarks>
/// Invoking this method is equivalent to calling Wait() on the <paramref name="task" /> if it is not completed.
/// </remarks>
public static void ThrowIfFaulted(Task task)
{
task.GetAwaiter().GetResult();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.IO;
using System.Linq;
using Microsoft.AspNet.Mvc.Core;
using Microsoft.AspNet.Mvc.Internal;
using Microsoft.Framework.DependencyInjection;

namespace Microsoft.AspNet.Mvc.Rendering
Expand Down Expand Up @@ -99,7 +100,8 @@ public string Render()
using (view as IDisposable)
{
var viewContext = new ViewContext(_viewContext, viewEngineResult.View, _viewData, writer);
viewEngineResult.View.RenderAsync(viewContext).Wait();
var renderTask = viewEngineResult.View.RenderAsync(viewContext);
TaskHelper.ThrowIfFaulted(renderTask);
return writer.ToString();
}
}
Expand Down
58 changes: 58 additions & 0 deletions test/Microsoft.AspNet.Mvc.Core.Test/Internal/TaskHelperTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading.Tasks;
using Xunit;

namespace Microsoft.AspNet.Mvc.Internal
{
public class TaskHelperTest
{
[Fact]
public void ThrowIfFaulted_DoesNotThrowIfTaskIsNotFaulted()
{
// Arrange
var task = Task.FromResult(0);

// Act and Assert
Assert.DoesNotThrow(() => TaskHelper.ThrowIfFaulted(task));
}

[Fact]
public void ThrowIfFaulted_ThrowsIfTaskIsFaulted()
{
// Arrange
var message = "Exception message";
var task = CreatingFailingTask(message);

// Act and Assert
var ex = Assert.Throws<Exception>(() => TaskHelper.ThrowIfFaulted(task));
Assert.Equal(message, ex.Message);
}

[Fact]
public void ThrowIfFaulted_ThrowsFirstExceptionWhenAggregateTaskFails()
{
// Arrange
var message = "Exception message";
var task = Task.Run(async () =>
{
await Task.WhenAll(CreatingFailingTask(message),
CreatingFailingTask("different message"));
});

// Act and Assert
var ex = Assert.Throws<Exception>(() => TaskHelper.ThrowIfFaulted(task));
Assert.Equal(message, ex.Message);
}

private static Task CreatingFailingTask(string message)
{
return Task.Run(() =>
{
throw new Exception(message);
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Mvc.Rendering;
using Moq;
Expand Down Expand Up @@ -235,5 +236,29 @@ public void DisplayFor_FindsModel_EvenIfNullOrEmpty(string propertyValue)
// Assert
Assert.Empty(result.ToString());
}

[Fact]
public void DisplayFor_DoesNotWrapExceptionThrowsDuringViewRendering()
{
// Arrange
var expectedMessage = "my exception message";
var model = new DefaultTemplatesUtilities.ObjectTemplateModel { Property1 = "Test string", };
var view = new Mock<IView>();
view.Setup(v => v.RenderAsync(It.IsAny<ViewContext>()))
.Returns(Task.Run(() =>
{
throw new ArgumentException(expectedMessage);
}));
var viewEngine = new Mock<ICompositeViewEngine>();
viewEngine
.Setup(v => v.FindPartialView(It.IsAny<ActionContext>(), It.IsAny<string>()))
.Returns(ViewEngineResult.Found("test-view", view.Object));
var helper = DefaultTemplatesUtilities.GetHtmlHelper(model, viewEngine.Object);
helper.ViewData["Property1"] = "ViewData string";

// Act and Assert
var ex = Assert.Throws<ArgumentException>(() => helper.DisplayFor(m => m.Property1));
Assert.Equal(expectedMessage, ex.Message);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Mvc.Rendering;
using Microsoft.AspNet.Testing;
Expand Down Expand Up @@ -386,5 +387,29 @@ public void EditorFor_FindsModel_EvenIfNullOrEmpty(string propertyValue)
"<input class=\"text-box single-line\" id=\"Property1\" name=\"Property1\" type=\"text\" value=\"\" />",
result.ToString());
}

[Fact]
public void EditorFor_DoesNotWrapExceptionThrowsDuringViewRendering()
{
// Arrange
var expectedMessage = "my exception message";
var model = new DefaultTemplatesUtilities.ObjectTemplateModel { Property1 = "Test string", };
var view = new Mock<IView>();
view.Setup(v => v.RenderAsync(It.IsAny<ViewContext>()))
.Returns(Task.Run(() =>
{
throw new FormatException(expectedMessage);
}));
var viewEngine = new Mock<ICompositeViewEngine>();
viewEngine
.Setup(v => v.FindPartialView(It.IsAny<ActionContext>(), It.IsAny<string>()))
.Returns(ViewEngineResult.Found("test-view", view.Object));
var helper = DefaultTemplatesUtilities.GetHtmlHelper(model, viewEngine.Object);
helper.ViewData["Property1"] = "ViewData string";

// Act and Assert
var ex = Assert.Throws<FormatException>(() => helper.EditorFor(m => m.Property1));
Assert.Equal(expectedMessage, ex.Message);
}
}
}

0 comments on commit 9c4df46

Please sign in to comment.