From 9c4df4606f812e384c0221ab0239ec371b02fc54 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 20 Aug 2014 14:12:04 -0700 Subject: [PATCH] Modify TemplateRenderer to use ThrowIfFaulted instead of Task.Wait Fixes #782 --- .../Internal/TaskHelper.cs | 25 ++++++++ .../Rendering/Html/TemplateRenderer.cs | 4 +- .../Internal/TaskHelperTest.cs | 58 +++++++++++++++++++ .../Rendering/DefaultDisplayTemplatesTests.cs | 25 ++++++++ .../Rendering/DefaultEditorTemplatesTests.cs | 25 ++++++++ 5 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/Microsoft.AspNet.Mvc.Core/Internal/TaskHelper.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/Internal/TaskHelperTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/TaskHelper.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/TaskHelper.cs new file mode 100644 index 0000000000..92721c8aa7 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/TaskHelper.cs @@ -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 +{ + /// + /// Utility methods for dealing with . + /// + public static class TaskHelper + { + /// + /// Throws the first faulting exception for a task which is faulted. It preserves the original stack trace when + /// throwing the exception. + /// + /// + /// Invoking this method is equivalent to calling Wait() on the if it is not completed. + /// + public static void ThrowIfFaulted(Task task) + { + task.GetAwaiter().GetResult(); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/TemplateRenderer.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/TemplateRenderer.cs index d4cca1a18e..b045a3bfe6 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/TemplateRenderer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/TemplateRenderer.cs @@ -8,6 +8,7 @@ using System.Globalization; 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 @@ -99,7 +100,8 @@ namespace Microsoft.AspNet.Mvc.Rendering 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(); } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Internal/TaskHelperTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/TaskHelperTest.cs new file mode 100644 index 0000000000..a37c88d731 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/TaskHelperTest.cs @@ -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(() => 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(() => TaskHelper.ThrowIfFaulted(task)); + Assert.Equal(message, ex.Message); + } + + private static Task CreatingFailingTask(string message) + { + return Task.Run(() => + { + throw new Exception(message); + }); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultDisplayTemplatesTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultDisplayTemplatesTests.cs index cb7155f3ce..98e762f599 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultDisplayTemplatesTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultDisplayTemplatesTests.cs @@ -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; @@ -235,5 +236,29 @@ namespace Microsoft.AspNet.Mvc.Core // 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(); + view.Setup(v => v.RenderAsync(It.IsAny())) + .Returns(Task.Run(() => + { + throw new ArgumentException(expectedMessage); + })); + var viewEngine = new Mock(); + viewEngine + .Setup(v => v.FindPartialView(It.IsAny(), It.IsAny())) + .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(() => helper.DisplayFor(m => m.Property1)); + Assert.Equal(expectedMessage, ex.Message); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultEditorTemplatesTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultEditorTemplatesTests.cs index 066c2325ba..3881b80085 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultEditorTemplatesTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/DefaultEditorTemplatesTests.cs @@ -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; @@ -386,5 +387,29 @@ Environment.NewLine; "", 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(); + view.Setup(v => v.RenderAsync(It.IsAny())) + .Returns(Task.Run(() => + { + throw new FormatException(expectedMessage); + })); + var viewEngine = new Mock(); + viewEngine + .Setup(v => v.FindPartialView(It.IsAny(), It.IsAny())) + .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(() => helper.EditorFor(m => m.Property1)); + Assert.Equal(expectedMessage, ex.Message); + } } } \ No newline at end of file