advanced-cms / advanced-reviews

This is an Open Source add-on that improves the reviewing process and lets external users to view & review content items or whole projects without the need to access the Edit Mode. Created and maintained by Bartosz Sekuła and Grzegorz Wiecheć
Apache License 2.0
17 stars 15 forks source link

Stack Overflow when viewing page previews #255

Open TreSmith opened 2 weeks ago

TreSmith commented 2 weeks ago

This is a confirmed issue on the latest version of advanced-reviews version 1.3.7

We have content editors that were running into issues when trying to create preview links on a few pages in the site. I've pulled down the advanced reviews package locally to inspect why the stack is getting so large to the point of crashing the server.

I think I've boiled down the issue to an infinite loop happening when GetChildrenWithReviews() is called in the ReviewsContentLoader.cs file. For some reason it's being called with a ContentReference with an id of 2. This causes the code to recursively call itself. After some research, I found this ID of 2 is a reference to the Waste Basket in Optimizely. However we're not referencing the Waste Basket on the page anywhere.

TreSmith commented 2 weeks ago

This chunk of code here in GetChildrenWithReviews() line 80-84

        var referenceWithoutVersion = contentLink.ToReferenceWithoutVersion();
        if (referenceWithoutVersion == ContentReference.WasteBasket)
        {
            return _contentLoader.GetChildren<T>(contentLink);
        }

calls .GetChildren(contentLink) when the ContentReference is a reference to the WasteBasket The GetChildren that is called is the override version found in the DraftContentLoader.cs file line 57

This chunk of code ends up looping until the server crashes

    public override IEnumerable<T> GetChildren<T>(ContentReference contentLink)
    {
        if (!_externalReviewState.IsInExternalReviewContext)
        {
            return _defaultContentLoader.GetChildren<T>(contentLink);
        }

        return _reviewsContentLoader().GetChildrenWithReviews<T>(contentLink).ToList();
    }
barteksekula commented 1 week ago

@TreSmith I have just pushed a fix. Would really appreciate if you could take this pre-release and gave it a shot. https://github.com/advanced-cms/advanced-reviews/actions/runs/11665152126

TreSmith commented 1 week ago

Sounds good I'll test it today, thanks for the quick fix!

TreSmith commented 1 week ago

After some testing looks the recent updates fixed the issue with causing stack overflow.

Quick question, I clicked the link you sent but I'm not super familiar with GitHub pipelines. How do I take it to prerelease? I can review the code if that's what you're referring to.

barteksekula commented 6 days ago

here is the artifact https://github.com/advanced-cms/advanced-reviews/actions/runs/11665152126/artifacts/2140971911

barteksekula commented 13 hours ago

@TreSmith Did you manage to test that version I sent you last week?