umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.33k stars 2.64k forks source link

Schedule publish jobs cannot get value of a multi-lingual item and nothing ever publishes #16224

Open kmeilander opened 3 weeks ago

kmeilander commented 3 weeks ago

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

13.2.2

Bug summary

Schedule publish jobs cannot get value of a multi-lingual item, and if this occurs, it throws an exception in the schedule publish flow and prevents any item from being published.

This actually came about with an Umbraco Commerce issue with the stock handler, but after further testing it looks like it's a commerce issue.

Specifics

Here is the full error for Umbraco Commerce: System.ArgumentNullException: Value cannot be null. (Parameter 'key1') at Umbraco.Cms.Infrastructure.PublishedCache.Property.GetValue(String culture, String segment) at Umbraco.Extensions.PublishedPropertyExtension.Value[T](IPublishedProperty property, IPublishedValueFallback publishedValueFallback, String culture, String segment, Fallback fallback, T defaultValue) at Umbraco.Commerce.Cms.Helpers.PublishedContentHelper.GetPropertyValue[T](IPublishedProperty prop, String culture, Fallback fallback) at Umbraco.Commerce.Cms.Helpers.PublishedContentHelper.GetNodePropertyValue[T](IPublishedContent productNode, IPublishedElement productVariantNode, String propertyAlias, String languageIsoCode, Func2 ancestorSelector, Boolean recursive, Func2 hasValueCheck, VariantFallbackBehavior variantFallbackBehavior) at Umbraco.Commerce.Cms.Finders.UmbracoPublishedContentStoreFinder.FindStoreByPublishedContent(IPublishedContent node) at Umbraco.Commerce.Cms.Finders.UmbracoPublishedContentStoreFinder.FindStore(Int32 nodeId) at Umbraco.Commerce.Cms.Services.UmbracoStoreService.FindStoreByNodeId(Int32 nodeId) at Umbraco.Commerce.Cms.PropertyEditors.Stock.UmbracoStockSynchronizer.Synchronize(IContent content, SynchronizeAttempt attempt) at Umbraco.Commerce.Cms.Events.Notification.Handlers.SynchronizeUmbracoStockOnSaving.Handle(ContentSavingNotification notification) at Umbraco.Cms.Core.Events.INotificationHandler1.Handle(IEnumerable1 notifications) at Umbraco.Cms.Core.Events.EventAggregator.PublishCore[TNotification](IEnumerable1 allHandlers, IEnumerable1 notifications) at Umbraco.Cms.Core.Events.NotificationHandlerWrapperImpl1.Handle[TNotification,TNotificationHandler](IEnumerable1 notifications, ServiceFactory serviceFactory, Action2 publish) at Umbraco.Cms.Core.Events.EventAggregator.PublishNotifications[TNotification,TNotificationHandler](IEnumerable1 notifications) at Umbraco.Cms.Core.Events.EventAggregator.Publish[TNotification,TNotificationHandler](IEnumerable1 notifications) at Umbraco.Cms.Core.Events.ScopedNotificationPublisher1.PublishCancelable(ICancelableNotification notification) at Umbraco.Cms.Core.Services.ContentService.PerformScheduledPublishingRelease(DateTime date, List1 results, EventMessages evtMsgs, Lazy1 allLangs) at Umbraco.Cms.Core.Services.ContentService.PerformScheduledPublish(DateTime date) at Umbraco.Cms.Infrastructure.BackgroundJobs.Jobs.ScheduledPublishingJob.RunJobAsync() ​ I have also isolated the issue using the Umbraco Demo Store. See https://github.com/umbraco/Umbraco.Commerce.DemoStore/compare/main...kmeilander:Umbraco.Commerce.DemoStore:breaking-publish-demo

I initially thought it was a Umbraco Commerce bug, but it appears to be an Umbraco issue.

Steps to reproduce

  1. Setup a site with a node that allows vary by culture. Add a property. Setup multiple languages in the install.
  2. Create a new ContentSavingNotification handler.
  3. From the handler, load the content and attempt to get the property value.
  4. In the Backoffice, create a new item of that doc type. And set any content item to be scheduled in the future.

Expected result / actual result

EXPECTED: That content item will be published when the time comes.

ACTUAL: The handler will be called with the schedule publish, and will throw an exception trying to load the value. And since the exception fails in the handle the content is never published. This will break for all scheduled published items.

github-actions[bot] commented 3 weeks ago

Hi there @kmeilander!

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 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:

kmeilander commented 3 weeks ago

I looked into things a little bit and i think it has something to do with the published request not being able to find the culture for the item.

The handler works fine for the Umbraco user, but the Umbraco Schedule publish handler it fails for some reason. But if i specify the culture in the GetValue("en") call, it works as expected (see my Umbraco Demo store fork)

(i think this is where it's going) https://github.com/umbraco/Umbraco-CMS/blob/b0b2b597403bb381c23ff47f2b1ffc8aeaab1dc0/src/Umbraco.PublishedCache.NuCache/Property.cs#L271