rhythmagency / formulate

An advanced form builder for Umbraco.
MIT License
92 stars 50 forks source link

CSV download fails after a large volume of submissions #199

Closed TFAstudio closed 3 years ago

TFAstudio commented 3 years ago

Hey folks,

We're load testing a solution right now, and after a few thousand records are added via a data handler, the CSV Download interface fails in the back-office. Yes it works after just a few records are added. Yes the records in the database seem fine and we can page through them in the back-office. The front end form does not fail. Just the CSV download back-office process after the testing is done and we try and download the data.

We do not know exactly what the record tipping point is. But it seems volume related. Umbraco v7.15.7, Formulate v2.6

Any ideas?

The CSV download loads an error in the same window:

An error has occurred. Object reference not set to an instance of an object. System.NullReferenceException at System.Object.GetType() at CsvHelper.CsvWriter.WriteField[T](T field) at formulate.app.Controllers.StoredDataDownloadController.GenerateCsvOfFormSubmissions(Form form) at formulate.app.Controllers.StoredDataDownloadController.DownloadCsvExport(GetCsvExportRequest request) at lambda_method(Closure , Object , Object[] ) at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ActionExecutor.<>c__DisplayClass6_2.b__2(Object instance, Object[] methodParameters) at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ExecuteAsync(HttpControllerContext controllerContext, IDictionary`2 arguments, CancellationToken cancellationToken) --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Controllers.ApiControllerActionInvoker.d__1.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Web.Http.Filters.ActionFilterAttribute.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Controllers.ActionFilterResult.d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Controllers.ExceptionFilterResult.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Web.Http.Controllers.ExceptionFilterResult.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Dispatcher.HttpControllerDispatcher.d__15.MoveNext()
Nicholas-Westby commented 3 years ago

I don't see anything in the code that would cause a failure on CSV export after a few thousand records. Is there some code you can share that creates the data so I can reproduce it locally?

Looking at the stack trace, perhaps one of the field values is null and maybe the CSV writer doesn't like to write null values? There was at least at some point exactly that bug (though I think it was fixed for the version of CsvHelper we're using): https://github.com/JoshClose/CsvHelper/issues/275

Here's the body of the Formulate function that writes the values in case you want to review it for issues:

/// <summary>
/// Generates a CSV of the form submissions for the specified form.
/// </summary>
/// <param name="form">
/// The form to generate the CSV for.
/// </param>
/// <returns>
/// The CSV.
/// </returns>
private byte[] GenerateCsvOfFormSubmissions(Form form)
{

    // Variables.
    var formId = form.Id;
    var db = ApplicationContext.DatabaseContext.Database;
    var filteredFields = form.Fields.Where(x => !x.IsTransitory);
    var fieldIds = filteredFields.Select(x => x.Id).ToArray();
    var fieldsById = filteredFields.ToDictionary(x => x.Id, x => x);
    var extractedValues = new List<SubmissionExport>();

    // Query database for form submissions.
    var query = new Sql().Select("*").From("FormulateSubmission")
        .Where("FormId = @0", formId).OrderByDescending("CreationDate");
    var entries = db.Fetch<FormulateSubmission>(query).ToList();

    // Extract field values from the database entries.
    foreach (var entry in entries)
    {
        var extractedRow = new Dictionary<Guid, string>();
        var exportRow = new SubmissionExport()
        {
            Submission = entry,
            Values = extractedRow
        };
        extractedValues.Add(exportRow);
        var values = GetValuesForFields(entry.DataValues);
        var filenames = GetValuesForFiles(entry.FileValues);
        foreach (var fieldId in fieldIds)
        {
            var value = default(string);
            if (values.TryGetValue(fieldId, out value))
            {
            }
            else if (filenames.TryGetValue(fieldId, out value))
            {
            }
            else
            {
                value = null;
            }
            extractedRow[fieldId] = value;
        }
    }

    // Store to a CSV.
    var config = new CsvHelper.Configuration.CsvConfiguration()
    {
        HasHeaderRecord = true,
        QuoteAllFields = true
    };
    using (var memStream = new MemoryStream())
    {

        // Open stream/writer.
        using (var textWriter = new StreamWriter(memStream))
        using (var writer = new CsvWriter(textWriter, config))
        {
            // Write headers.
            writer.WriteField("Creation Date");
            writer.WriteField("URL");
            writer.WriteField("Page ID");
            foreach (var fieldId in fieldIds)
            {
                var field = fieldsById[fieldId];
                writer.WriteField(field.Name);
            }
            writer.NextRecord();

            // Write values.
            foreach (var row in extractedValues)
            {
                writer.WriteField(row.Submission.CreationDate);
                writer.WriteField(row.Submission.Url);
                writer.WriteField(row.Submission.PageId);
                foreach (var fieldId in fieldIds)
                {
                    var value = default(string);
                    if (!row.Values.TryGetValue(fieldId, out value))
                    {
                        value = null;
                    }
                    writer.WriteField(value);
                }
                writer.NextRecord();
            }

        }

        // Return CSV as a byte array.
        return memStream.ToArray();

    }

}
TFAstudio commented 3 years ago

The site was load tested using Jmeter. We could share the test script if you thought it might be helpful. Or we could export the Formulate data table if that might prove revealing.

All the records seem to be intact, complete and visible in the Formulate back-office Submissions tools.

Nicholas-Westby commented 3 years ago

A backup of the database or export of the submissions table, as well as the accompanying Formulate JSON files, would be of use.

TFAstudio commented 3 years ago

Have you got an email address I can send those elements to? Thanks, AE

Nicholas-Westby commented 3 years ago

You can include a link to the download in a form submission here: https://www.nicholaswestby.com/contact

Nicholas-Westby commented 3 years ago

The exception occurs on this line:

null-reference-exception

And indeed the page IDs are mostly null:

null-page-ids

Normally, Formulate would not create a submission with a null page ID, so this seems like an issue with how you're generating these entries.

Still, it's odd that it's throwing an error since it shouldn't matter if the page ID is null (it is retrieved as a nullable integer, so this possibility was accounted for). If I were to guess, I'd say the problem is with this function in CsvHelper:

get-converter

Maybe the type converter used by a nullable integer does weird things when converting a null integer.

Perhaps a newer version of CsvHelper would help with that. Not sure.

If you'd like to submit a pull request that fixes this, please do so and I'll make sure it is included in an upcoming release.

TFAstudio commented 3 years ago

There were NULL values after all. #facepalm You nailed it from the start. The Jmeter script used for load-testing was bypassing the normal submission process (so it could track success) and must not be passing those values.

That's something we can ensure is fixed for the next time we do load-testing. In the meantime, I imagine this behaviour is closely tied to the https://github.com/rhythmagency/formulate/issues/163 issue previously reported. Different cause, same result. If we have time to fix, we'll be sure to submit a pull request.

You are a master at investigating these occasional issues! Thank you!