When using an IContentLastChanceFinder, the content finder is triggered before resolving the redirects

[MiguelLopez6]

We are using Skybrud 4.0.6 on a Umbraco 10.4.0 install. We've migrated from a v8 install where we were already using the package. Now on v10 we encountered this issue.

When using a custom IContentLastChanceFinder together with skybrud redirects, the TryFindContent method of the content finder is triggered before the redirects are resolved.

{
    services.AddUmbraco(_env, _config)
        .AddBackOffice()
        .AddWebsite()
        .AddComposers()
        .SetContentLastChanceFinder<My404ContentFinder>()
        .Build();
}

public class My404ContentFinder : IContentLastChanceFinder
{

        public Task<bool> TryFindContent(IPublishedRequestBuilder contentRequest)
        {
                // This code here is triggered before the redirect is being resolved
        }
}

[abjerner]

Hi @MiguelLopez6

Based on your description, I'd say this is working as intended/expected.

If you're having problem with redirects not working, it's likely because your logic in your 404 content finder doesn't set a proper 404 status code - which then results in redirects module not kicking in.

[MiguelLopez6]

Hi @abjerner , thanks for your quick response.

The redirects work, but only after adding some extra code inside the content finder, so we check if there is a redirect before returning a 404 result page,

//Check if redirect exists in Skybrud Redirects
if (redirectsService.GetRedirectByUri(request.Uri) != null)
{
      return Task.FromResult(false);
}
else 
{
      request.SetPublishedContent(notFoundNode);
      return Task.FromResult(true);
}

whereas in V8 the redirects were resolved first and then kicked the content finder.

To me, it didn't look right, as the IContentLastChanceFinder should be the "last chance".

So I believe what you said is the expected behaviour and we are not actually doing any "hacky" workaround?

Thanks a lot.

[abjerner]

I can't tell for sure, but you might have done something in Umbraco 8 that you weren't supposed to. The "last chance" name comes from Umbraco, and I see that more as related to finding content than redirects. Ideally your content finder shouldn't have to know about any redirects.

With the last snippet that you've shared, it seems that if a redirect exists, you don't set your own 404 page, which I guess would fall back to Umbraco's default 404 page instead - which correctly sets the status code. When setting your custom 404 page, it seems that you're not setting an explicit status code, meaning it'll most likely be 200 - meaning that the redirects module won't do anything.

So the key is making sure that your 404 page actually responds with a 404 status code. You've only posted portions of the code for your content finder, but try replacing:

//Check if redirect exists in Skybrud Redirects
if (redirectsService.GetRedirectByUri(request.Uri) != null)
{
      return Task.FromResult(false);
}
else 
{
      request.SetPublishedContent(notFoundNode);
      return Task.FromResult(true);
}

with:

request.SetPublishedContent(notFoundNode);
request.SetResponseStatus(404);
return Task.FromResult(true);

[MiguelLopez6]

Thanks @abjerner. Inside the IContentLastChanceFinder, the response status is already 404, and as soon as the code hits "SetPublishedContent", it will return the specified 404 page, so it seems we still need the if clause.

[abjerner]

@MiguelLopez6 what's the status code when your custom 404 page hits the browser?

Can you share an extended example of your content finder implementation. The more code you'll be able to share, to easier it will be for me to set up a test case.

[MiguelLopez6]

Thanks @abjerner.

The custom 404 page shows a status of 200.

This is the full code of our custom 404 content finder:

public class PageNotFoundFinder : IContentLastChanceFinder
{
    private readonly IFileService fileService;
    private readonly IUmbracoContextAccessor umbracoContextAccessor;

    public PageNotFoundFinder(IFileService fileService, IUmbracoContextAccessor umbracoContextAccessor)
    {
        this.fileService = fileService;
        this.umbracoContextAccessor = umbracoContextAccessor;
    }

    /// <summary>
    /// Used to find the correct 404 page when a page cannot be found in Umbraco
    /// </summary>
    /// <param name="contentRequest">The current Umbraco context</param>
    /// <returns>The 404 page</returns>
    public Task<bool> TryFindContent(IPublishedRequestBuilder request)
    {
        // Check request cannot be found
        if (request.PublishedContent == null)
        {
            //try to get the umbraco context
            if (!umbracoContextAccessor.TryGetUmbracoContext(out var umbracoContext))
            {
                return Task.FromResult(false);
            }

            // don't show a redirect for sitemap.xml
            if (request.Uri.AbsolutePath.ToLower().Contains("sitemapxml"))
            {
                var firstNode = umbracoContext.Content?.GetAtRoot().FirstOrDefault();
                if (firstNode != null)
                {
                    var template = fileService.GetTemplate("SitemapXml");
                    request.SetPublishedContent(firstNode); // The content
                    request.SetTemplate(template);
                    return Task.FromResult(true);
                }
            }

            //Check if redirect exists in Skybrud Redirects
        if (redirectsService.GetRedirectByUri(request.Uri) != null)
        {
            return Task.FromResult(false);
        }

            var domain = request.Domain;
            if (domain != null)
            {
                var home = umbracoContext.Content?.GetById(domain.ContentId);

                if (home != null)
                {
                    // Get the 404 node
                    var notFoundNode = home.FirstChild<PageNotFound>();

                    if (notFoundNode != null)
                    {
                        // Set the node to be the not found node
                        request.SetPublishedContent(notFoundNode);
                        request.SetResponseStatus(404);
                        return Task.FromResult(true);
                    }
                }
            }
            else
            {
                var root = umbracoContext.Content?.GetAtRoot().FirstOrDefault();
                var pageNotFoundGlobal = root?.FirstChild<PageNotFound>();

                if (pageNotFoundGlobal != null)
                {
            // Set the node to be the not found node
            request.SetPublishedContent(pageNotFoundGlobal);
                    request.SetResponseStatus(404);
                    return Task.FromResult(true);
                }
            }
        }

        return Task.FromResult(false);
    }
}

As you see, we have the following lines: if (redirectsService.GetRedirectByUri(request.Uri) != null) to return false. Otherwise, the contentfinder will return the 404 page and will not process any redirects.

[abjerner]

The custom 404 page shows a status of 200.

This is why. The status code has to be 404. This is the main issue.

I haven't yet had the time to test the code for your content finder, but when skimming through the code, I think you're setting the status code correctly, so I'm a bit unsure why the status code is then 200.

Is there why any chance some logic elsewhere that sets the status code back to 200?

[abjerner]

Hi again @MiguelLopez6

I just tried setting up your content finder in my local Umbraco test installation, but without your redirects check, and everything works fine. In my case, the 404 page also responds with a 404 status code like expected.

So as I wrote in my last comment, is there why any chance some logic elsewhere that sets the status code back to 200? It's my bet that this is the real issue, and not this package.

[ghost]

Hi @abjerner,

What's the reason to handle the redirects after everything else? Why not find a possible redirect before all kinds of custom logic is executed? For instance, if I request a page '/some-page' and for this page is a redirect configured, does it matter what custom logic adds up to the request?

Thanks

[abjerner]

Hi @djanjicek

It's a design choice we made when initially developing the package. The general idea is that if something already exists at the requested URL, we don't want the package to do anything. This is why the redirects logic only kicks in when the request has a 404 status code.

For one, this is what fit our use case at the time - and still does.

And second, an important thing to consider is performance. When looking for a redirect, the package hits the database. So there is a big difference doing database lookups for all request opposed to only doing lookups for 404 requests.

One could argue that some of this could be mitigated by a cache layer, so the package doesn't hit the database for each request. This will however add more complexity to the package, and so far not something we've felt was necessary.

With the status of idea, I did create an issue for forced redirects a while back for the Umbraco 8 version of the package. But as you can see from the issue, it has hardly had any feedback.

[ghost]

Thanks @abjerner. Right now we set a custom 404 page, the 404 status, and the published content in a custom controller. If I’ve seen correctly, the fact of the published content would not result in a valid redirect. I believe the RedirectService checks for the published content and http status.