[Bug]: NullReferenceException thrown in BaseChart::LimitDataSets method

[BravermanMillimanCRS]

Blazorise Version

1.6

What Blazorise provider are you running on?

Bootstrap5

Link to minimal reproduction or a simple code snippet

Unfortunately, creating a code snippet to reproduce this would take quite a lot of time. Suffice it to say, during the execution of a Blazor page OnInitializedAsync method, it is possible for the LicenseChecker to be null.

The offending code in our app is this:

await _lineChart.AddDatasetsAndUpdate(chartData);

The exception gets thrown in Blazorise.Charts.BaseChart:

106 private void LimitDataSets(params TDataSet[] datasets)
107 {
108     if (!datasets.IsNullOrEmpty() && LicenseChecker.GetChartsRowsLimit().HasValue)
109     {
110         foreach (TDataSet val in datasets)
111         {
112             val.Data = LimitData(val.Data);
113         }
114     }
115 }

At line 108, LicenseChecker is null.

Steps to reproduce

Execute Chart.AddDatasetsAndUpdate while in the OnInitializedAsync phase of a page load.

What is expected?

The expected behavior is that no exception will be thrown in the BaseChart class.

What is actually happening?

A NullReferenceException is thrown from the BaseChart class.

What browsers do you see the problem on?

Chrome, Microsoft Edge

Any additional comments?

There's a very simple fix. Change line 108 in BaseChart either to do null propagation or check for a null LicenseChecker. Or even better, because the LimitData method winds up calling GetChartRowsLimit in a loop, it would make more sense to check it once get rid of LimitData altogether:

private void LimitDataSets(params TDataSet[] datasets)
{
    if (datasets.IsNullOrEmpty() || LicenseChecker is null) return;

    var rowsLimit = LicenseChecker.GetChartsRowsLimit();
    if (!rowsLimit.HasValue) return;

    foreach (TDataSet val in datasets)
    {
        val.Data = val.Data.Take(rowsLimit).ToList();
    }
}

There is another code smell, of course, in that the code executes a method and uses it like a property. The method BlazoriseLicenseProvider::GetDataGridRowsLimit (ultimately the source of LicenseChecker's data) could be refactored.

[David-Moreira]

The LicenseChecker is not supposed to be null. And I'd like it if you could be so kind as to provide a repro, would that be possible?

But I guess we can null check and still maintain some sane defaults for our business needs in this case in order to keep users as unaffected as possible by any side cases.

The possible issue with the loop that you talk about shouldn't be a problem, as the method is just doing simple logic and accessing internal properties and unless the loop would be done a million times I'm sure it isn't a performance concern, but thanks for raising it.

BlazoriseLicenseProvider::GetDataGridRowsLimit I'm not sure what your suggestion is here? Is is qualified as a method, because there is some logic going on under the covers, even if simple to decide what limit it is, also the value could be different at different times in the application, i.e the license is still initializing. Why do you think it should be a property?

Again, thank you for your feedback.

[BravermanMillimanCRS]

I totally understand the need for sample code, but after looking at our code to figure out how to do a minimal POC for this issue, I realized it would take several hours. We're a commercial shop; I couldn't spend that much billable time on it. And since the fix was so easy, it seemed like a disproportionate effort for something that hasn't been reported before.

We're loading the chart inside a control inside a page, with a lot of other things going on to render the page. At the point where the NRE gets thrown, it looks like the license checker hasn't finished initializing. We've found that a lot of things that "shouldn't be null" are, in fact, null at some points during a page's lifecycle, particularly when the page has been decomposed into multiple controls, and everything is running asynchronously. Plus, this happens a lot more often when we're debugging locally than in our dev/test environment, so it might even be related to how Visual Studio's debugger works.

Thanks for taking a look at it anyway.