umbraco / Umbraco.Forms.Issues

Public issue tracker for Umbraco Forms
29 stars 0 forks source link

Unable to use async actions on a custom controller for forms #86

Closed sussexrick closed 2 years ago

sussexrick commented 5 years ago

When using custom "hijacked" controllers in Umbraco we can use the async/await pattern to improve scalability. This works for most Umbraco document types, but it doesn't work with forms in Umbraco Forms 7.0.6 on Umbraco 7.13.2 (or earlier versions).

Reproduction

Create a new form with one field, and a minimal document type/template to host it. Use 'Form' as the document type alias. Add a minimal hijacked custom controller:

using System.Web.Mvc;
using Umbraco.Web.Models;
using Umbraco.Web.Mvc;

namespace UmbracoFormsLatest.Controllers
{
    public class FormController : RenderMvcController
    {
        public override ActionResult Index(RenderModel model)
        {
            return CurrentTemplate(model);
        }
    }
}

Test the form and it will submit.

Now change the controller to async. This requires the use of the new keyword because the base method returns ActionResult but the async version needs to return Task<ActionResult>:

using System.Threading.Tasks;
using System.Web.Mvc;
using Umbraco.Web.Models;
using Umbraco.Web.Mvc;

namespace UmbracoFormsLatest.Controllers
{
    public class FormController : RenderMvcController
    {
        public new async Task<ActionResult> Index(RenderModel model)
        {
            // await something...

            return CurrentTemplate(model);
        }
    }
}

Now test the form again and, as soon as you submit you get the following error in the Umbraco logs (and sometimes on screen, depending on error settings):

System.InvalidOperationException: The asynchronous action method 'Index' returns a Task, which cannot be executed synchronously.

The relevant part of the call stack for Umbraco is:

Umbraco.Web.Mvc.UmbracoPageResult.ExecuteControllerAction(ControllerContext context, IController controller) +104
Umbraco.Web.Mvc.UmbracoPageResult.ExecuteResult(ControllerContext context) +388

It's possible to work around this for single page forms by selecting another page in the "Go to URL" part of the submit workflow. On a multi-page form the same error occurs when navigating to page 2, and there is no obvious workaround apart from putting all fields onto one page.

Please can something be done in this method to support async actions that return Task<ActionResult>.

nul800sebastiaan commented 5 years ago

As you have noticed, this is currently not supported and I don't think there are plans to support it at this time.

It looks like we're rendering the form using a ChildAction and ChildAction's don't support async as far as I know, so it might be impossible to implement this.

Shazwazza commented 5 years ago

I don't think Forms is the issue here, it really comes down to that we aren't properly supporting async actions for route hijacking. Using the new keyword probably shouldn't have worked to begin with. We'd need to properly support async actions from top to bottom.

Would probably need an IndexAsync action method to override, we'd need to modify the action resolver stuff to look for declared suffixed Async action names, and ensure we use the async mvc actions where appropriate.

AndyButland commented 2 years ago

Closing this one as, as noted above, the resolution would need to happen in the CMS.