Search code examples
azureasynchronousazure-sql-databaseblazor-server-side

Async call causing application to crash after calling the db


The title isn't great. I'm not clear on the actual problem but I think its related to async/sync calls to the db as the application is loading. I'm going to give a lot of my internal thinking so you can help me understand where I am going wrong. I have a Blazor application, deployed to an Azure app service, using Azure SQL db. It stops responding while its loading. By "loading" I mean I can see in the network tab of the browser Inspector where it gets the CSS, but then nothing is displayed, it's just a white screen.

There are no useful errors in Azure Insights, db logs or anywhere else I can find. I see some queries to the db and then there are a stack of "task cancelled" exceptions which leads me to believe something is happening that is killing the app and no errors are bubbling up.

Of course it works fine on my local machine using a SQL Express db. I attached to remote debug the app from VS and I found the last query it ran in the Output window. I put a breakpoint on that db call in my repository and as soon as I stepped through it, it crashed. I now know this is related to a service that loads some properties I need throughout the app. I'm using dependency injection to make those properties available. Here's the thread where I got that idea: How can I intialize a base property that needs an injected service in a Blazor app?

Here is the service I created to hold those properties. The relevant line in all this is _currentChangePeriod = Task.Run(async () => await AdminDomain.GetCurrentChangePeriodAsync()).Result;. That is the db call that is causing the app to crash.

public class HelperService : IHelperService
{
    private IAdminDomain? AdminDomain { get; set; }
    private bool _inChangePeriod;
    bool IHelperService.InChangePeriod
    {
        get => _inChangePeriod;
    }

    private ChangePeriodDTO _currentChangePeriod;
    ChangePeriodDTO IHelperService.CurrentChangePeriod
    {
        get => _currentChangePeriod;
    }
    private ChangePeriodDTO _nextChangePeriod;
    ChangePeriodDTO IHelperService.NextChangePeriod
    {
        get => _nextChangePeriod;
    }

    public HelperService(IAdminDomain adminDomain) 
    { 
        AdminDomain = adminDomain;

        _currentChangePeriod = Task.Run(async () => await AdminDomain.GetCurrentChangePeriodAsync()).Result;
        _inChangePeriod = _currentChangePeriod.EndDate >= DateTime.UtcNow.Date ? true : false;
        if (!_inChangePeriod)
        {
            var changePeriods = AdminDomain.GetAllChangePeriodsAsync().Result;
            _nextChangePeriod = changePeriods.Where(c => c.StartDate > DateTime.UtcNow.Date).OrderBy(x => x.StartDate)
                .DefaultIfEmpty(new ChangePeriodDTO())
                .First();
        }
    }
}

and it's instantiated in Program.cs: builder.Services.AddScoped<IHelperService, HelperService>();. If you noticed, I put the call to get the data in the constructor. Two reasons for that: 1) I was hoping the data would be retrieved when its instantiated and 2) I couldn't figure out any other way to do it. I am reading about the Factory Pattern but I can't get my head around that yet. If it would even help?

I'm injecting it in Main.razor.cs [Inject] private IHelperService? HelperService { get; set; } and calling it from OnInitialized, which is the first place I need the properties ShowChangeMessage = !HelperService.InChangePeriod;.

I initially created the Helper Service with a synchronous call in the constructor _currentChangePeriod = AdminDomain.GetCurrentChangePeriodAsync().Result;. If I'm misusing synchronous and asynchronous, I apologize. I'm going by my current understanding.

After I changed it to async, which you can see in the class definition, it works when I use the VS debugger attached to the Azure app service. It still won't work just running the app in Azure.

I started to look into lazy loading. I thought that would delay calling the db until I need the data. I understand the concept but not sure if it will help.


Solution

  • (picking up on your comment) it's OK, you can still do this with just one DB call for all properties.

    I'll show here how you can use a single Lazy call to do one DB operation, and then make the results available through several "properties" (actually methods). Without having to call the DB again.

    I've modified your code as minimally as possible to achieve that here. NB that the Interface has changed to return Task Methods, instead of simple property getters. This is because you have to accept somewhere that there'll be some async wait operations, whereas properties are meant to return quickly without any logic. Code first, then I'll explain a bit:

    Tweak to the interface to have Methods instead of properties:

        public interface IHelperService
        {
            Task<bool> InChangePeriodAsync();
            Task<ChangePeriodDTO> CurrentChangePeriodAsync();
            Task<ChangePeriodDTO> NextChangePeriodAsync();
        }
    

    ...and keeping your class looking as similar as possible, but making it async safe with a single DB call:

        public class HelperService : IHelperService
        {
            async Task<bool> IHelperService.InChangePeriodAsync()
            {
                return (await _dbOperation.Value).InChangePeriod;
            }
    
            async Task<ChangePeriodDTO> IHelperService.CurrentChangePeriodAsync()
            {
                return (await _dbOperation.Value).CurrentChangePeriod;
            }
    
            async Task<ChangePeriodDTO> IHelperService.NextChangePeriodAsync()
            {
                return (await _dbOperation.Value).NextChangePeriod;
            }
    
            private readonly Lazy<Task<(ChangePeriodDTO CurrentChangePeriod, bool InChangePeriod, ChangePeriodDTO NextChangePeriod)>> _dbOperation;
    
            public HelperService(IAdminDomain adminDomain) 
            { 
                _dbOperation = new Lazy<Task<(ChangePeriodDTO CurrentChangePeriod, bool InChangePeriod, ChangePeriodDTO NextChangePeriod)>>(async () =>
                {
                    var currentChangePeriod = await adminDomain.GetCurrentChangePeriodAsync();
                    var inChangePeriod = currentChangePeriod.EndDate >= DateTime.UtcNow.Date ? true : false;
                    if (!inChangePeriod)
                    {
                        var changePeriods = await adminDomain.GetAllChangePeriodsAsync();
                        var nextChangePeriod = changePeriods.Where(c => c.StartDate > DateTime.UtcNow.Date).OrderBy(x => x.StartDate)
                            .DefaultIfEmpty(new ChangePeriodDTO())
                            .First();
    
                        return (currentChangePeriod, inChangePeriod, nextChangePeriod);
                    }
    
                    return (currentChangePeriod, inChangePeriod, null);
                });
            }
        }
    

    So what's going on?

    1. On Construction. The Constructor now just creates an async function definition, as a lambda. It doesn't execute it. The async function is stored in the Lazy variable, and not executed immediately.

    2. When a property (method) is accessed for the first time. As soon as one of the public properties (actually now methods) is called, the Lazy object creates the async function. Assuming the calling method is propertly awaiting this Task, the resulting Task object is stored in the Lazy along with its Result.

    3. When another property (method) is accessed. Assuming that the caller did actually await the Task in step 2, then the Result will already exist. The response is returned without another DB query.

    Benefits of this approach

    • Constructor returns immediately without an delay waiting for the DB, which is a better pattern to follow (see docs linked in comments)
    • Any exceptions thrown by the async DB operations will now be easier to trap and log.

    Some slightly ugly things to note

    To keep the code changes as minimal as possible, I hacked a few changes in that don't look pretty, but it was just so you could compare more easily. Things I'd tidy are:

    • I'm returning quite a lengthy 3-part Tuple from the new async lambda. There are neater ways.
    • I've got two return statements from the lambda (depending if inChangePeriod) which doesn't look neat; for tidyness you'd be better declaring the return variables outside the if and then returning once.
    • You can replace the DefaultIfEmpty with a more efficient solution:
        var nextChangePeriod = changePeriods.Where(c => c.StartDate > DateTime.UtcNow.Date).OrderBy(x => x.StartDate)
             .FirstOrDefault() ?? new ChangePeriodDTO();
    

    Note I haven't tested this in a Blazor app etc, so I'd be interested to know if it does actually solve your problem!