Error on startup after 12.1 upgrade

richarth commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

12.1

Bug summary

We had a v11 site which we upgraded successfully to v12. We've just tried to upgrade the site to v12.1 and are encountering the following error when trying to start the site:

System.AggregateException: 'Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: Umbraco.Cms.Persistence.EFCore.Migrations.IMigrationProvider Lifetime: Singleton ImplementationType: Umbraco.Cms.Persistence.EFCore.SqlServer.SqlServerMigrationProvider': Unable to resolve service for type 'Microsoft.EntityFrameworkCore.IDbContextFactory1[Umbraco.Cms.Persistence.EFCore.UmbracoDbContext]' while attempting to activate 'Umbraco.Cms.Persistence.EFCore.SqlServer.SqlServerMigrationProvider'.) (Error while validating the service descriptor 'ServiceType: Umbraco.Cms.Persistence.EFCore.Migrations.IMigrationProvider Lifetime: Singleton ImplementationType: Umbraco.Cms.Persistence.EFCore.Sqlite.SqliteMigrationProvider': Unable to resolve service for type 'Microsoft.EntityFrameworkCore.IDbContextFactory1[Umbraco.Cms.Persistence.EFCore.UmbracoDbContext]' while attempting to activate 'Umbraco.Cms.Persistence.EFCore.Sqlite.SqliteMigrationProvider'.) (Error while validating the service descriptor 'ServiceType: Umbraco.Cms.Infrastructure.Migrations.IEFCoreMigrationExecutor Lifetime: Singleton ImplementationType: Umbraco.Cms.Persistence.EFCore.EfCoreMigrationExecutor': Unable to resolve service for type 'Microsoft.EntityFrameworkCore.IDbContextFactory1[Umbraco.Cms.Persistence.EFCore.UmbracoDbContext]' while attempting to activate 'Umbraco.Cms.Persistence.EFCore.SqlServer.SqlServerMigrationProvider'.) (Error while validating the service descriptor 'ServiceType: Umbraco.Cms.Core.Events.INotificationAsyncHandler1[Umbraco.Cms.Infrastructure.Migrations.Notifications.DatabaseSchemaAndDataCreatedNotification] Lifetime: Transient ImplementationType: Umbraco.Cms.Persistence.EFCore.Composition.EFCoreCreateTablesNotificationHandler': Unable to resolve service for type 'Microsoft.EntityFrameworkCore.IDbContextFactory1[Umbraco.Cms.Persistence.EFCore.UmbracoDbContext]' while attempting to activate 'Umbraco.Cms.Persistence.EFCore.SqlServer.SqlServerMigrationProvider'.) (Error while validating the service descriptor 'ServiceType: Umbraco.Cms.Core.Events.INotificationAsyncHandler1[Umbraco.Cms.Core.Notifications.UnattendedInstallNotification] Lifetime: Transient ImplementationType: Umbraco.Cms.Persistence.EFCore.Composition.EFCoreCreateTablesNotificationHandler': Unable to resolve service for type 'Microsoft.EntityFrameworkCore.IDbContextFactory1[Umbraco.Cms.Persistence.EFCore.UmbracoDbContext]' while attempting to activate 'Umbraco.Cms.Persistence.EFCore.SqlServer.SqlServerMigrationProvider'.)'`

I don't know if anybody has any ideas?

Specifics

No response

Steps to reproduce

Upgrade a v11 site to v12, upgrade to v12.1 in Visual Studio then attempt to run the site.

Expected result / actual result

The site should run and the upgrade process should run.

_This item has been added to our backlog AB#31902_

github-actions[bot] commented 1 year ago

Hi there @richarth!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
We'll replicate the issue to ensure that the problem is as described.
We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

arknu commented 1 year ago

We're seeing this as well upgrading from 12.0 to 12.1

rammi987 commented 1 year ago

See the same as well upgrading from 12.0.1 to 12.1.0

rammi987 commented 1 year ago

Hello @richarth and @arknu I solved the issue for my project going from 12.0.1 to 12.1.0.

Can you guys inside your project find the Startup.cs file and inside that file try to search for the pisce of code .AddDeliveryApi() If you missing this line inside the Startup.cs file then search for .AddWebsite() and the add the .AddDeliveryApi() as in the next of the pipe chain so you end up with this in the method ConfigureServices

var umbraco = services.AddUmbraco(_env, _config)
                .AddBackOffice()
                .AddWebsite()
                .AddDeliveryApi()
                .AddComposers()
                .Build();

and the try to rerun the website. It fix my error.

If you have the .AddDeliveryApi() inside your startup file, please also return then i can debug futher

kjac commented 1 year ago

Hi everyone. That's really interesting 🤔

I can reproduce this on a fresh 12.1 install by removing .AddDeliveryApi() from ConfigureServices in Startup. That's ... unexpected to say the least.

We'll look into this 🔍

kjac commented 1 year ago

To clarify for anyone just tuning in. ConfigureServices in Startup must contain .AddDeliveryApi() - like so:

public void ConfigureServices(IServiceCollection services)
{
    services.AddUmbraco(_env, _config)
        .AddBackOffice()
        .AddWebsite()
        .AddDeliveryApi()
        .AddComposers()
        .Build();
}

Adding this line does NOT turn on the Delivery API on its own, so it will not cause unexpected data leaks. The Delivery API must be explicitly enabled in appsettings.json. See the docs for more info.

kjac commented 1 year ago

...thanks for the quick investigation efforts, @rammi987 👍

richarth commented 1 year ago

Hi,

Thanks @rammi987. I can confirm the site was missing .AddDeliveryApi(). Adding that has gotten the site running again.

rammi987 commented 1 year ago

@kjac found some inside the Umbraco.Cms.Api.Delivery.DependencyInjection.UmbracoBuilderExtensions add in the UmbracoDbContext Context.

So deleting the line AddDeliveryApi() will acully do the the EF Core context of type UmbracoDbContext never exiting in the DI. So when Umbraco is starting is hit the composer https://github.com/umbraco/Umbraco-CMS/blob/contrib/src/Umbraco.Cms.Persistence.EFCore/Composition/UmbracoEFCoreComposer.cs

that dependens on the UmbracoDbContext but again never added to the ServiceProvider scope.

rammi987 commented 1 year ago

Even after Upgrade NEVER delete the AddDeliveryApi() line, the exception will return 💀

thomas-jf commented 1 year ago

Yes, adding .AddDeliveryApi() in Startup fixes the issue (updating from 12.0.1 to 12.1.0)

JasonElkin commented 1 year ago

I've seen @Migaroez PR #14667 so Startup.cs may not be the best place for AddDeliveryApi() anyway, but...

It would be a good idea to have some kind of check/safeguard in place to sound the klaxons when Startup.cs (or Program.cs) gets modified as it will always throw a spanner in the works for upgrades - changing it should be avoided unless absolutely necessary.

If it is necessary then I'd expect to see that reflected in the release notes, and in the version specific upgrade docs and for everyone that knows about it to make a lot of noise about it before the release.

nikcio commented 1 year ago

To clarify this issue stems from the https://github.com/umbraco/Umbraco-CMS/blob/contrib/src/Umbraco.Cms.Persistence.EFCore/Composition/UmbracoEFCoreComposer.cs

Because here some services are added that rely on the DBContext but for some reason, it was decided that the DBContext should be added to the services explicitly in the Delivery API project 🤔

@kjac This was also one of the things I mentioned here: https://github.com/umbraco/Umbraco-CMS/pull/14449#issuecomment-1637044620 😅

And in this PR: https://github.com/umbraco/Umbraco-CMS/pull/14563

Ambertvu commented 1 year ago

Just a minor addition, I encountered this issue as well, but only when running in Production mode, locally it would run and upgrade fine.

nikolajlauridsen commented 1 year ago

Heya, this has been fixed in #14678 which is @nikcio's PR but without the breaking change 😄 So big thank you for that 😄

This change has already been released in 12.1.1, which is available on Nuget, so I'll go ahead and close this issue, thanks again.

umbraco / Umbraco-CMS