Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack Overflow when viewing page previews #255

Open
TreSmith opened this issue Oct 29, 2024 · 11 comments
Open

Stack Overflow when viewing page previews #255

TreSmith opened this issue Oct 29, 2024 · 11 comments

Comments

@TreSmith
Copy link

TreSmith commented Oct 29, 2024

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
Copy link
Author

TreSmith commented Oct 29, 2024

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
Copy link
Member

@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

@barteksekula barteksekula reopened this Nov 4, 2024
@TreSmith
Copy link
Author

TreSmith commented Nov 4, 2024

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

@TreSmith
Copy link
Author

TreSmith commented Nov 6, 2024

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
Copy link
Member

@barteksekula
Copy link
Member

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

@TreSmith
Copy link
Author

Hey sorry haven't had a chance to push it to our developer site. I tested the package locally and it seemed to no longer cause a stack overflow issue. However it seems to load and not stop loading. I let it run for maybe 20 minutes and it didn't seem to load properly. It wasn't crashing the local server so the original issue seems to be fixed. I'll maybe have to dig a bit deeper into the loading issue when I have a bit more time. Could you push the latest update to the nuget server?

@barteksekula
Copy link
Member

@TreSmith I'd appreciate stacktrace. I can't think of any other issue than some circular dependency between some content items causing this. Do you have any custom events connected to any of IContentEvents?

@barteksekula
Copy link
Member

version 1.3.8 will soon be available on nuget.optimizely.com

@TreSmith
Copy link
Author

TreSmith commented Nov 15, 2024

I can check, off the top of my head I know we have pages that have "related pages" that on publish we make sure both pages have each other listed as related. This property is a List ContentReference type

@TreSmith
Copy link
Author

TreSmith commented Nov 25, 2024

Hopping back into this, sorry for the delay. I got more time to look into this issue and should have more information today. I found a page that is still having the stack overflow issue. I'll be looking at the cause of this here today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants