mariusmuntean / ChartJs.Blazor

Brings Chart.js charts to Blazor
https://www.iheartblazor.com/
MIT License
677 stars 151 forks source link

Problem placing two charts on one page #98

Closed MajorMajor0 closed 4 years ago

MajorMajor0 commented 4 years ago

Describe the bug

When two charts are on one razor page, a null reference exception is thrown.

Which Blazor project type is your bug related to?

Which charts does this bug apply to?

Scatter

To Reproduce

A project to reproduce the problem is here: https://github.com/MajorMajor0/NateSucksAtPunchout

Build and surf to charts and you will get the error. Comment out Charts.Razor lines 8, 32-34 (removes the first chart) and the problem goes away. Restore those and comment out lines 12, 37-39 (removes the second chart) and the problem goes away again.

The problem comes up if both charts are displayed but either chart works perfectly alone

Expected behavior

Two charts on one page with no exception.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context / logging

warn: Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer[100] Unhandled exception rendering component: Object reference not set to an instance of an object. System.NullReferenceException: Object reference not set to an instance of an object. at ChartJs.Blazor.Charts.ChartJsScatterChart.BuildRenderTree(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.ComponentBase.<.ctor>b6_0(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.Rendering.ComponentState.RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment renderFragment) at Microsoft.AspNetCore.Components.RenderTree.Renderer.RenderInExistingBatch(RenderQueueEntry renderQueueEntry) at Microsoft.AspNetCore.Compnents.RenderTree.Renderer.ProcessRenderQueue() Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer: Warning: Unhandled exception rendering component: Object reference not set to an instance of an object.

System.NullReferenceException: Object reference not set to an instance of an object. at ChartJs.Blazor.Charts.ChartJsScatterChart.BuildRenderTree(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.ComponentBase.<.ctor>b6_0(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.Rendering.ComponentState.RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment renderFragment) at Microsoft.AspNetCore.Components.RenderTree.Renderer.RenderInExistingBatch(RenderQueueEntry renderQueueEntry) at Microsoft.AspNetCore.Components.RenderTree.Renderer.ProcessRenderQueue() fail: Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost[111] Unhandled exception in circuit 'EA5dGGe5y4h7J4Phs9eWubVcfHETPXjE_JxLuZDkZtY'. System.NullReferenceException: Object reference not set to an instance of an object. at ChartJs.Blazor.Charts.ChartJsScatterChart.BuildRenderTree(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.ComponentBase.<.ctor>b6_0(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.Rendering.ComponentState.RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment renderFragment) at Microsoft.AspNetCore.Components.RenderTree.Renderer.RenderInExistingBatch(RenderQueueEntry renderQueueEntry) at Microsoft.AspNetCore.Components.RenderTree.Renderer.ProcessRenderQueue() Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost: Error: Unhandled exception in circuit 'EA5dGGe5y4h7J4Phs9eWubVcfHETPXjE_JxLuZDkZtY'.

System.NullReferenceException: Object reference not set to an instance of an object. at ChartJs.Blazor.Charts.ChartJsScatterChart.BuildRenderTree(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.ComponentBase.<.ctor>b6_0(RenderTreeBuilder builder) at Microsoft.AspNetCore.Components.Rendering.ComponentState.RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment renderFragment) at Microsoft.AspNetCore.Components.RenderTree.Renderer.RenderInExistingBatch(RenderQueueEntry renderQueueEntry) at Microsoft.AspNetCore.Components.RenderTree.Renderer.ProcessRenderQueue()

Joelius300 commented 4 years ago

Thanks for contacting us.

The error is in your code. The way you're using Task.Run in Charts.Razor.cs is very dangerous and causes undefined behaviour like you've just experienced yourself. Instead make the method synchronous or even better use ToListAsync() when fetching your players.
Avoid using Task.Run just to make something async unless you really know what you're doing. Especially in a web context it can be dangerous to use. Also I know this is a repro project but there are lots of redundancies in your code. If it was more organized, you might've been able to find the bug yourself :)

Meaning instead of

await Task.Run(() =>
{
    var players = context.Players.Include(x => x.Finishes).ThenInclude(x => x.YearNavigation).ToList();

    foreach (Player player in players)
    {
        ...
    }
});

you do

var players = await context.Players.Include(x => x.Finishes).ThenInclude(x => x.YearNavigation).ToListAsync();

foreach (Player player in players)
{
    ...
}

Below is the patch file for the fix (you can apply it with git apply).

diff --git a/NateSucksAtPunchout/Pages/Charts.Razor.cs b/NateSucksAtPunchout/Pages/Charts.Razor.cs
index cc63f29..d29d277 100644
--- a/NateSucksAtPunchout/Pages/Charts.Razor.cs
+++ b/NateSucksAtPunchout/Pages/Charts.Razor.cs
@@ -24,28 +24,26 @@ namespace NateSucksAtPunchout.Pages
        {
            List<ScatterDataset> returner = new List<ScatterDataset>();

-           await Task.Run(() =>
-           {
-               var players = context.Players.Include(x => x.Finishes).ThenInclude(x => x.YearNavigation).ToList();
+           var players = await context.Players.Include(x => x.Finishes).ThenInclude(x => x.YearNavigation).ToListAsync();

-               foreach (Player player in players)
+           foreach (Player player in players)
+           {
+               string color = ColorUtil.RandomColorString();
+               ScatterDataset sds = new ScatterDataset
                {
-                   string color = ColorUtil.RandomColorString();
-                   ScatterDataset sds = new ScatterDataset
-                   {
-                       BackgroundColor = color,
-                       BorderColor = color,
-                       ShowLine = true,
-                       LineTension = 0,
-                       PointRadius = 5,
-                       PointHitRadius = 5,
-                       PointHoverRadius = 8
-                   };
-                   sds.Label = player.FirstName;
-                   sds.Data = player.GetCumulativeWinnings();
-                   returner.Add(sds);
-               }
-           });
+                   BackgroundColor = color,
+                   BorderColor = color,
+                   ShowLine = true,
+                   LineTension = 0,
+                   PointRadius = 5,
+                   PointHitRadius = 5,
+                   PointHoverRadius = 8
+               };
+
+               sds.Label = player.FirstName;
+               sds.Data = player.GetCumulativeWinnings();
+               returner.Add(sds);
+           }

            return returner;
        }
@@ -59,28 +57,25 @@ namespace NateSucksAtPunchout.Pages
        {
            List<ScatterDataset> returner = new List<ScatterDataset>();

-           await Task.Run(() =>
-           {
-               var players = context.Players.Include(x => x.Finishes).ThenInclude(x => x.YearNavigation).ToList();
+           var players = await context.Players.Include(x => x.Finishes).ThenInclude(x => x.YearNavigation).ToListAsync();

-               foreach (Player player in players)
+           foreach (Player player in players)
+           {
+               string color = ColorUtil.RandomColorString();
+               ScatterDataset sds = new ScatterDataset
                {
-                   string color = ColorUtil.RandomColorString();
-                   ScatterDataset sds = new ScatterDataset
-                   {
-                       BackgroundColor = color,
-                       BorderColor = color,
-                       ShowLine = true,
-                       LineTension = 0,
-                       PointRadius = 5,
-                       PointHitRadius = 5,
-                       PointHoverRadius = 8
-                   };
-                   sds.Label = player.FirstName;
-                   sds.Data = player.GetNormalizedCumulativeWinnings(normalizedTo);
-                   returner.Add(sds);
-               }
-           });
+                   BackgroundColor = color,
+                   BorderColor = color,
+                   ShowLine = true,
+                   LineTension = 0,
+                   PointRadius = 5,
+                   PointHitRadius = 5,
+                   PointHoverRadius = 8
+               };
+               sds.Label = player.FirstName;
+               sds.Data = player.GetNormalizedCumulativeWinnings(normalizedTo);
+               returner.Add(sds);
+           }

            return returner;
        }
diff --git a/NateSucksAtPunchout/Pages/Charts.razor b/NateSucksAtPunchout/Pages/Charts.razor
index ea12092..cd09a6a 100644
--- a/NateSucksAtPunchout/Pages/Charts.razor
+++ b/NateSucksAtPunchout/Pages/Charts.razor
@@ -5,7 +5,7 @@

 <h2>Charts</h2>
 <div>
-   @*<ChartJsScatterChart @ref="cumulativeWinningsChart" Config="@cumulativeWinningsConfig" Width="600" Height="300" />*@
+   <ChartJsScatterChart @ref="cumulativeWinningsChart" Config="@cumulativeWinningsConfig" Width="600" Height="300" />
 </div>

 <div>
@@ -29,9 +29,9 @@
    {
        using (DataModel.BigGameStatsContext context = new DataModel.BigGameStatsContext())
        {
-           //cumulativeWinningsConfig = LoadConfig("Cumulative Winnings");
-           //var cumulativeWinningDataSets = await GetCumulativeWinningDataSetsAsync(context);
-           //cumulativeWinningsConfig.Data.Datasets.AddRange(cumulativeWinningDataSets);
+           cumulativeWinningsConfig = LoadConfig("Cumulative Winnings");
+           var cumulativeWinningDataSets = await GetCumulativeWinningDataSetsAsync(context);
+           cumulativeWinningsConfig.Data.Datasets.AddRange(cumulativeWinningDataSets);

            double norm = 200;
            normalizedWinningsConfig = LoadConfig($"Winnings Normalized to ${norm}");

Happy coding