umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
Other
4.49k stars 2.69k forks source link

Umb 8.2 - Publish with Descendants errors #6698

Closed KevinJump closed 5 years ago

KevinJump commented 5 years ago

if you have an unpublished node and you attempt to publish the parent node with the Publish with Descendants option, you get an error. (most of the time, not always!)

Reproduction

Bug summary

Publishing a parent node with 'Publish with descendants' can cause an error:

Server error: Contact administrator, see log for full details.
No more children.

Steps to reproduce

This can be reproduced on a blank install with the starter kit, but it doesn't always display the error (so might take two or three goes).

  1. NuGet install of Umbraco 8.2.0 ,

  2. installed via the 'install' (so with the starter kit)

  3. Unpublish the Todo list for this starter kit page image

  4. Attempt to publish with Descendants the 'About us page' image

This may work, fine, try steps 3 & 4 Again a few times and it will break.

  1. Once broken republishing the Todo List page and trying to publish descendants from About Us will still produce the result

via code, publishing a number of child nodes (but not all of them) and then attempting to publish the parent or another unpublished child node can also produce similar results - but this is a lot harder to reproduce for an issue

Expected result

The page should publish with no errors.

Actual result

The screenshot above and The logged error is :

Exception
Umbraco.Core.Exceptions.PanicException: No more children.
   at Umbraco.Web.PublishedCache.NuCache.ContentStore.AddTreeNodeLocked(ContentNode content, LinkedNode`1 parentLink) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 1035
   at Umbraco.Web.PublishedCache.NuCache.ContentStore.SetBranch(Int32 rootContentId, IEnumerable`1 kits) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 746
   at Umbraco.Web.PublishedCache.NuCache.PublishedSnapshotService.NotifyLocked(IEnumerable`1 payloads, Boolean& draftChanged, Boolean& publishedChanged) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\PublishedSnapshotService.cs:line 663
   at Umbraco.Web.PublishedCache.NuCache.PublishedSnapshotService.Notify(JsonPayload[] payloads, Boolean& draftChanged, Boolean& publishedChanged) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\PublishedSnapshotService.cs:line 602

I have also seen, but i am struggling to get a new installation to reproduce this one.

Umbraco.Core.Exceptions.PanicException: failed to get last child with id=-1
   at Umbraco.Web.PublishedCache.NuCache.ContentStore.GetRequiredLinkedNode(Int32 id, String description, Nullable`1 gen) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 828
   at Umbraco.Web.PublishedCache.NuCache.ContentStore.AddTreeNodeLocked(ContentNode content, LinkedNode`1 parentLink) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 986
   at Umbraco.Web.PublishedCache.NuCache.ContentStore.SetBranch(Int32 rootContentId, IEnumerable`1 kits) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 746
   at Umbraco.Web.PublishedCache.NuCache.PublishedSnapshotService.NotifyLocked(IEnumerable`1 payloads, Boolean& draftChanged, Boolean& publishedChanged) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\PublishedSnapshotService.cs:line 663

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

ronaldbarendse commented 5 years ago

Seems related to https://github.com/umbraco/Umbraco-CMS/issues/6419. And according to the comments on https://github.com/umbraco/Umbraco-CMS/issues/6450, a lot has been done to fix these issues in NuCache, although it might not have been enough 😉

stevemegson commented 5 years ago

I think these are reliable steps to reproduce both errors. For "No more children":

  1. Unpublish a "middle child"
  2. Save and publish it
  3. Publish it with descendants
  4. Repeat steps 2 and 3

For "failed to get last child":

  1. Unpublish a last child
  2. Save and publish it
  3. Publish it with descendants
  4. Repeat steps 2 and 3

It does seem to be related to resetting the sibling IDs in RemoveTreeNodeLocked. When the node is saved and published after unpublishing, the version with the sibling IDs set to -1 is found as the existing version, and its sibling IDs are copied to the new generation.

With the sibling IDs broken, when publishing with descendants we either fail to find the right place in the sort order as in #6419, or we copy the -1 from PreviousSiblingContentId to the parent's LastChildContentId in RemoveTreeNodeLocked.

Shazwazza commented 5 years ago

Thanks @stevemegson , i can replicate with these steps, now need to write tests to resolve. Thanks for your research!

Shazwazza commented 5 years ago

I have a test created that replicates the issue. What I find a bit strange is that in the ContentStore.SetBranch method, it first clears the branch with ClearBranchLocked which is fine, this essentially zero's out the new Gen for the branch since it's going to fill in these null values after that ... but it then calls RemoveTreeNodeLocked which is going to actually remove the root node of this particular branch for the current Gen (not the new one that is being created) which seems odd since this branch shouldn't be modified for the current Gen, only for the new Gen but this effectively makes this branch disapear for the current Gen. 🤔 I'm still investigating what changes are needed to fix this correctly.

zpqrtbnk commented 5 years ago

To begin with, IIRC ContentStore.Set is setting FirstChildContentId and NextSiblingContentId but not Last... and Previous....

Shazwazza commented 5 years ago

haha, yep, i just found that too and it fixes the issue https://github.com/umbraco/Umbraco-CMS/pull/6734

But ... removing the branch when refresh branch is called for the previous gen's i don't believe is correct.

clausjensen commented 5 years ago

@Shazwazza I've marked the PR ready for review and will merge it in. Apart from what you mention yourself here about modifying the previous gen branch, the changes look good - and I've confirmed it fixes the issue.

nul800sebastiaan commented 5 years ago

For people who need a quick fix before we can do a proper release of 8.2.1: attached is an 8.2.0 dll, with just this fix applied on top of it, you only need to replace this one dll and it should be fixed for you:

Umbraco.Web.dll.zip

enkelmedia commented 5 years ago

I have the same issue on 8.2.0,

In may case I just create a new child node from code

var node = _contentService.Create("New name", parentNode.Key, 'myDocTypeAlias');
// set som props
_contentService.SaveAndPublish(node, raiseEvents:false);

It worked the first time, but the 2nd time the code executed i get the error below, BUT. the DLL with the fix that @nul800sebastiaan provided fixed the problem. So I just wanted to add this for reference.

Umbraco.Core.Exceptions.PanicException: failed to get last child with id=-1
 at Umbraco.Web.PublishedCache.NuCache.ContentStore.GetRequiredLinkedNode(Int32 id, String description, Nullable`1 gen) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 828
 at Umbraco.Web.PublishedCache.NuCache.ContentStore.AddTreeNodeLocked(ContentNode content, LinkedNode`1 parentLink) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 986
shearer3000 commented 5 years ago

I get the "failed to get last child with id=-1" sporadically in 8.2.0 when save+publishing new nodes and/or trying to sort a newly created node within its siblings. Not sure on exact replication steps sorry, but following this ticket.

OksXen commented 5 years ago

Same problem here.

nul800sebastiaan commented 5 years ago

Umbraco 8.2.1 was just released @shearer3000 and @OksXen, make sure to upgrade and give it another go.

OksXen commented 5 years ago

Thanks @nul800sebastiaan! The upgrade went well and it fixed the '-1' error message issue.

shearer3000 commented 5 years ago

thanks @nul800sebastiaan all ok so far for me too after the upgrade :)

DanDiplo commented 4 years ago

I know this is closed, but I'm constantly getting this issue in 8.3.0 since I enabled variants. Seems to happen nearly every time I create a page or sort anything. Can't see any obvious pattern, apart from the content does still get created after the error, but the tree doesn't update.

clausjensen commented 4 years ago

@DanDiplo Hmm it must be a different problem (with the same symptoms however) since this was fixed (and confirmed fixed by others) in 8.2.1 and included in 8.3.0.

Could you create a new issue with a description of what you're seeing so we can look into that?

lukehook commented 4 years ago

@clausjensen @DanDiplo I'm experiencing this same issue in 8.5.2 - I too think it's related somehow to variants. Did another ticket ever get created? If not I shall make one now

DanDiplo commented 4 years ago

@lukehook No, I never reported an issue as I couldn't reproduce the error - it was too intermittent. But I agree I'm sure it's related to variants, especially when adding / editing properties on a document type.

clausjensen commented 4 years ago

Well if you come up with a fairly consistent way to reproduce we'll be happy to take a look at it 😃

lukehook commented 4 years ago

@DanDiplo are you perhaps running the external ModelsBuilder in DLL mode by chance? I've able to pretty consistently recreate the error in that scenario. It seems every time I'm deploying, the cache is breaking, possibly locking. I'll lock another issue in case it's unrelated

DanDiplo commented 4 years ago

@lukehook Actually, I wasn't. I need to look into it again, but my feeling was that it was "breaking" when I was adding or removing variant properties to existing document types, often via infinite editing.