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.48k stars 2.69k forks source link

XPath for 404 pages sometimes doesn't work in v13 #15582

Closed C0derPr0 closed 6 months ago

C0derPr0 commented 9 months ago

What type of issue is it?

Wrong documentation

What article/section is this about?

https://github.com/umbraco/UmbracoDocs/blob/main/13/umbraco-cms/tutorials/custom-error-page.md

Describe the issue

The documentation provided for Error404Collection no longer works.

Bad example:

{ "Umbraco": { "CMS": { "Content": { "Error404Collection": [ { "Culture": "default", "ContentXPath": "//errorPages[@nodeName='My cool error']" } ] } } } }

It returns the error System.InvalidOperationException: XPathValue must be an XPathNavigator or a string. As I understand it, XPath support was removed from v13.

alina-tincas commented 9 months ago

Hi @C0derPr0 πŸ‘‹

This has been previously brought to our attention that XPath is obsolete. After further talks with our developers it is confirmed that they are scheduled for removal in v14 and are currently working on an alternative. You can read more about this here.

Until then there is a warning on our documentation about this https://docs.umbraco.com/umbraco-cms/tutorials/custom-error-page and when v14 is out (this summer) the XPath examples will be updated as well: image

C0derPr0 commented 9 months ago

Hi @alina-tincas

Yes, but the example on the page that I referenced doesn’t work now. Why give a presently non-functional example "An XPath statement (example: //ErrorPages[@nodeName='My cool error'] ) and then say it will be removed in v14. It is misleading. It doesn't work now in v13. I just upgraded to 13 & the example that I’ve been using for years broke my error handler. You also say that the XPath statement is preferred for Umbraco Cloud. The documentation is wrong.

[coderPro.net animated logo] Brandon Osborne Chief Software Architect coderPro.nethttp://coderpro.net/ β€œDone Right the First Time!”

@.https://linkedin.com/in/coderpro @.https://www.facebook.com/coderproz [A close up of a logo Description automatically generated]http://twitter.com/coderProNet [Adobe Certified Developer - ColdFusion Badge]https://www.credly.com/badges/0ee0be46-da2b-4d0b-b0b4-d57c41b6a261/public_url [Umbraco Certified Expert] [WhiteHat Certified Secure Developer Badge]

From: Alina-Magdalena Tincas @.> Sent: Monday, January 15, 2024 5:16 AM To: umbraco/UmbracoDocs @.> Cc: Brandon Osborne @.>; Mention @.> Subject: Re: [umbraco/UmbracoDocs] Incorrect Documentation - Error404Collection - XPath Not Supported in v13 (Issue umbraco/Umbraco-CMS#15582)

Hi @C0derPr0https://github.com/C0derPr0 πŸ‘‹

This has been previously brought to our attention that XPath is obsolete. After further talks with our developers it is confirmed that they are scheduled for removal in v14 and are currently working on an alternative. You can read more about this herehttps://github.com/umbraco/UmbracoDocs/issues/5394#issuecomment-1755027287.

Until then there is a warning on our documentation about this https://docs.umbraco.com/umbraco-cms/tutorials/custom-error-page and when v14 is out (this summer) the XPath examples will be updated as well: image.png (view on web)https://github.com/umbraco/UmbracoDocs/assets/83591955/25d0a59d-ab45-41e9-8b57-1acf921bc144

β€” Reply to this email directly, view it on GitHubhttps://github.com/umbraco/Umbraco-CMS/issues/15582, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A2CI26FMCSFRIOG5YYWIFTLYOT6X3AVCNFSM6AAAAABBZIUMWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJRHAYDCNZTGU. You are receiving this because you were mentioned.Message ID: @.**@.>>

alina-tincas commented 9 months ago

Hi @CodeartIT I have confirmed with CMS team that XPath is still working on v13 so this sounds more like a bug, hence it will be transferred to CMS team's issue tracker πŸ’ͺ

liamlaverty commented 9 months ago

I've tested this on a fresh install of Umbraco 13.0.3. I was able to get the Error404Collection config to work with the following settings:

"Error404Collection": [
  {
    "Culture": "default",
    "ContentXPath": "//myErrorDocumentType[@nodeName='404 Page Not Found']"
  }

Where my 404 page's document type alias was myErrorDocumentType, and the node's name was 404 Page Not Found

C0derPr0 commented 9 months ago

I've tested this on a fresh install of Umbraco 13.0.3. I was able to get the Error404Collection config to work with the following settings:

"Error404Collection": [
  {
    "Culture": "default",
    "ContentXPath": "//myErrorDocumentType[@nodeName='404 Page Not Found']"
  }

Where my 404 page's document type alias was myErrorDocumentType, and the node's name was 404 Page Not Found

I did precisely the same thing (both on my upgraded version and a fresh install of 13.0.3) and the following code:

"Error404Collection": [
  {
    "Culture": "default",
    "ContentXPath": "//standardPage[@nodeName='Oops']"
  }
]

returns the default Umbraco error page. It only works when I use the ID or Guid. It works fine with ContentXPath in <= 12.x.

The following error is logged:

Could not parse xpath expression: //standardPage[@nodeName='Oops']
System.InvalidOperationException: XPathValue must be an XPathNavigator or a string.
   at Umbraco.Cms.Core.Xml.XPath.NavigableNavigator.MoveToFirstChildProperty()
   at Umbraco.Cms.Core.Xml.XPath.NavigableNavigator.MoveToFirstChild()
   at MS.Internal.Xml.XPath.XPathDescendantIterator.MoveNext()
   at MS.Internal.Xml.XPath.DescendantQuery.Advance()
   at MS.Internal.Xml.XPath.FilterQuery.Advance()
   at MS.Internal.Xml.XPath.XPathSelectionIterator.MoveNext()
   at Umbraco.Cms.Infrastructure.PublishedCache.ContentCache.GetSingleByXPath(XPathNodeIterator iterator)
   at Umbraco.Cms.Infrastructure.PublishedCache.ContentCache.GetSingleByXPath(Boolean preview, String xpath, XPathVariable[] vars)
   at Umbraco.Cms.Core.PublishedCache.PublishedCacheBase.GetSingleByXPath(String xpath, XPathVariable[] vars)
   at Umbraco.Cms.Infrastructure.PublishedContentQuery.ItemByXPath(String xpath, XPathVariable[] vars, IPublishedCache cache)
   at Umbraco.Cms.Infrastructure.PublishedContentQuery.ContentSingleAtXPath(String xpath, XPathVariable[] vars)
   at Umbraco.Cms.Core.Routing.NotFoundHandlerHelper.GetContentIdFromErrorPageConfig(ContentErrorPage errorPage, IEntityService entityService, IPublishedContentQuery publishedContentQuery, Nullable`1 domainContentId)

There are already a couple of posts on our.umbraco.com where developers say that XPath doesn't work in v13.

github-actions[bot] commented 9 months ago

Hi there @C0derPr0!

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:

andr317c commented 9 months ago

Hey! I tried following the documentation for using Xpath to create an Error404 page and it works without any issues on version 13.0.3 for me πŸ€”.

Here is a short GIF of what I have created for it to work: Xpath

wiedikerli commented 8 months ago

I have the same issue in Umbraco 13.1.0. image image

james-whittington1 commented 8 months ago

This is really frustrating. I have attached source link, made a custom 404 finder that mimics the built in finder, and found that it is throwing an error when the xpath navigator attempts to "look" at a rich text value. It is expecting a navigator or string, but receives an intermediate property value converter value instead. I have no idea how to work around this.

image

james-whittington1 commented 8 months ago

@andr317c what happens if you add a rich text datatype to the page in your test?

palmem29 commented 8 months ago

Following an Umbraco update from 12.3.6 to 13.0.3 I'm having the same issue. I was hopeful this was resolved in 13.1.1 but following the update I still have no luck. I did confirm that a fresh 13.1.1 install works just fine but I'm not in a position where I can start with a fresh 13.1.1 install and rebuild my project. Any traction with this issue?

cvocvo commented 8 months ago

For us, this post here from @marcemarc helped: https://our.umbraco.com/forum/using-umbraco-and-getting-started/113342-xpathvalue-must-be-an-xpathnavigator-or-a-string#comment-349382

For me it was the // in the XPATH, if I made it / path it worked

Thus this working setup for our multisite (literally changed from // to / and it fixed it. Oddly, our 404 pages were working on every site except one, but this slash change resolved the one that wasn't working and left the others functional as well.

        "Error404Collection": [
          {
            "Culture": "default",
            "ContentXPath": "$site/notFound[@nodeName='404']"
          }
        ]
palmem29 commented 7 months ago

@andr317c, if "The current implementation of XPath is suboptimal and will be removed entirely in a future version. It is currently obsolete and scheduled for removal in v14." will there be any way to target a 404 page outside of ID and GUID ID?

Currently, it's very nice to know that we put our 404 page at /Page-Not-Found. So we don't have to update it from project to project or instance to instance. I understand how to use the ID and GUID ID. Will those be our only options in Umbraco14?

TomChancer commented 7 months ago

Wanted to add on to this issue:

Also experiencing this on a Cloud based Baseline project that was upgraded from 12.3.7 to 13.2.2.

Original string was as follows: //error404[@nodeName='Error404']

Which returns the error in logs of: System.InvalidOperationException: XPathValue must be an XPathNavigator or a string.

Attempting the change mentioned by @cvocvo

Results in a new error due to the $site inclusion: System.Xml.XPath.XPathException: Namespace Manager or XsltContext needed. This query has a prefix, variable, or user-defined function.

Would like to know what the status is on this, as we have around 19 cloud baseline children with now broken 404's.

Zeegaan commented 6 months ago

Hello there πŸ‘‹ Mine seems to work just fine aswell πŸ˜• image image

C0derPr0 commented 6 months ago

Yes, it seems like the problems only occur when people upgrade from prior versions of Umbraco, but the folks at Umbraco ignore the error because this feature is going to be dropped in future versions. I've tested this myself on several sites, and it works fine in new builds, but it explodes every time I upgrade to v13 from any other version.

andr317c commented 6 months ago

Hey again,

Sorry for not getting back to you sooner, we've been quite busy with V14. I have tried to reproduce the issue once more. I set everything up in Version 12.1.0, and the error page worked as it should. Afterwards, I upgraded my project to Version 13.2.2. And the error page still works πŸ˜•. I also tried using an RTE on the Error Page, but I still had no issues.

This is what I have in my appsettings:

"Error404Collection": [
          {
            "Culture": "default",
            "ContentXPath": "//errorPages[@nodeName='My cool error']"
          }
        ],

I'm not really sure how to reproduce this issue, Could you possibly provide the exact steps you followed, starting from a clean install up to encountering the problem? Any additional insights would be greatly appreciated!

nul800sebastiaan commented 6 months ago

FYI: I tried as well with an upgrade from 12.3.6 and it's still working for me too. Could it be that your XPath expression is not right somehow? I tried the error page being on level 2 (under "Home") as well as on level 3 (under "Home" > "About us") to make sure we didn't break the // part of the XPath selector.

Of course if we broke it, it needs to be fixed but as noted a couple times above, we need a way to reproduce the problem. I can't see anything obvious in the changes between 12 and 13 either that could've broken this, so for now there's nowhere to start without a reproduction of the problem.

OwainJ commented 6 months ago

I ran into this on my new v13.2.2 site, where my 404 error page is located as a child of my Homepage. //errorPage[@nodeName='Page not found'] - doesn't work $site/errorPage[@nodeName='Page not found'] - does work

This is the error that shows in my logs if I use the //

Could not parse xpath expression: "//errorPage[@nodeName='Page not found']"
--
System.InvalidOperationException: XPathValue must be an XPathNavigator or a string.    
at Umbraco.Cms.Core.Xml.XPath.NavigableNavigator.MoveToFirstChildProperty()    
at Umbraco.Cms.Core.Xml.XPath.NavigableNavigator.MoveToFirstChild()    
at MS.Internal.Xml.XPath.XPathDescendantIterator.MoveNext()   
at MS.Internal.Xml.XPath.DescendantQuery.Advance()    
at MS.Internal.Xml.XPath.FilterQuery.Advance()   
at MS.Internal.Xml.XPath.XPathSelectionIterator.MoveNext()    
at Umbraco.Cms.Infrastructure.PublishedCache.ContentCache.GetSingleByXPath(XPathNodeIterator iterator)    
at Umbraco.Cms.Infrastructure.PublishedCache.ContentCache.GetSingleByXPath(Boolean preview, String xpath, XPathVariable[] vars)    
at Umbraco.Cms.Core.PublishedCache.PublishedCacheBase.GetSingleByXPath(String xpath, XPathVariable[] vars)    
at Umbraco.Cms.Infrastructure.PublishedContentQuery.ItemByXPath(String xpath, XPathVariable[] vars, IPublishedCache cache)    at Umbraco.Cms.Infrastructure.PublishedContentQuery.ContentSingleAtXPath(String xpath, XPathVariable[] vars)    
at Umbraco.Cms.Core.Routing.NotFoundHandlerHelper.GetContentIdFromErrorPageConfig(ContentErrorPage errorPage, IEntityService entityService, IPublishedContentQuery publishedContentQuery, Nullable`1 domainContentId)

I have 2 languages on my site, I just tried a mixed setup, where one culture was set to use $site/ and the other was set to use //, the 404 with the // failed, and the one with $site/ worked fine.

nul800sebastiaan commented 6 months ago

That might be it! I'll try multilingual!

nul800sebastiaan commented 6 months ago

Still no luck. I first tried with the 404 page not being multilingual, upgraded that to 13.2.2 (see zip, folder Issue15582-1322). Then made the 404 multi-lingual (in appSettings, and added multilingual options to the root node > node & hostnames).

Both going to https://localhost:44335/nl/efojpwefj and https://localhost:44335/en/wqojorjw AND also https://localhost:44335/fjpweqwef give me the correct results in both versions of the sites. I've zipped them both up so people can give it a go and get to a different result. Very strange all this.

Issue15582.zip

nul800sebastiaan commented 6 months ago

Note: when running each site you might want to do a dotnet clean in between, modelsbuilder gets confused since it's a site on the same port but with a different version.

nul800sebastiaan commented 6 months ago

And one more note to be extra clear, the config looks like:

        "Error404Collection": [ 
          { "Culture": "en-US", "ContentXPath": "//errorPages[@nodeName='My cool error']" },
          { "Culture": "nl-NL", "ContentXPath": "//errorPages[@nodeName='Mijn coole error']" } 
        ]

Where 'My cool error' and 'Mijn coole error' are the exact names of the pages in the CMS.

OwainJ commented 6 months ago

@nul800sebastiaan I downloaded your .zip and ran the v13.2.2 project, and 404 worked perfectly as expected.

So I decided to create my own vanilla Umbraco project to see if I could re-create the issue I'm seeing in my client's project, these are the steps I followed:

  1. Created a new project using this package script writer setup: https://psw.codeshare.co.uk/?InstallUmbracoTemplate=true&UmbracoTemplateVersion=11.4.1&Packages=uSync%7C13.2.0&UserEmail=admin%40example.com&ProjectName=404TestOjo&CreateSolutionFile=true&SolutionName=404TestOjo&UseUnattendedInstall=true&DatabaseType=SQLite&UserPassword=1234567890&UserFriendlyName=Administrator&IncludeStarterKit=true&StarterKitPackage=clean&OnelinerOutput=false&TemplateName=Umbraco.Templates&TemplateVersion=13.2.2&RemoveComments=false
  2. I then renamed the premade error page to "Page Not Found"
  3. Added a 404 collection
    "Error404Collection": [
     {
          "Culture": "default",
          "ContentXPath": "//error[@nodeName='Page not found']"
     }
    ]
  4. Navigated to a page that doesn't exists https://localhost:44356/sfefse
  5. And it showed me the default Umbraco 404.
  6. Checked the logs, and I can see the same Could not parse xpath expression: "//error[@nodeName='Page not found']" error that I am seeing on my client v13 project.

GIF of my Vanilla project (only package is uSync): Weird 404 issue

And here's .zip of that project (media files removed due to github upload limit): 404TestOjo.zip

BrionyMcKenz commented 6 months ago

I've been using "Error404Collection": [ { "Culture": "default", "ContentXPath": "//notFound404[@nodeName='Page not found']" } ] locally and it's worked fine starting on v13, I've upgraded each minor version and it's still working fine. However, I just did a deployment to Umbraco Cloud and it's not working in the Cloud environment. Weirdly it is broken for my colleague when he runs locally so is there maybe a cache issue or something that making it look like it's working for me in my local environment?

The documentation states to use XPath for Cloud because the node ids can change, is there an alternative?

alina-tincas commented 6 months ago

@BrionyMcKenz from v14 the alternative for ContentXPath is to use IContentLastChanceFinder, which can be used in v13 as well. You can see an example of how to use IContentLastChanceFinder here.

palmem29 commented 6 months ago

Thank you, @alina-tincas!

nul800sebastiaan commented 6 months ago

@OwainJ Yes, now we're talking! Thanks very much! With a reproduction in place it wasn't too difficult to find the code that caused the problem. I've left a more extensive description of the problem (plus a fix) in PR https://github.com/umbraco/Umbraco-CMS/pull/16121

Thanks again, that was really helpful!

anas1987 commented 6 months ago

Hey again,

Sorry for not getting back to you sooner, we've been quite busy with V14. I have tried to reproduce the issue once more. I set everything up in Version 12.1.0, and the error page worked as it should. Afterwards, I upgraded my project to Version 13.2.2. And the error page still works πŸ˜•. I also tried using an RTE on the Error Page, but I still had no issues.

This is what I have in my appsettings:

"Error404Collection": [
          {
            "Culture": "default",
            "ContentXPath": "//errorPages[@nodeName='My cool error']"
          }
        ],

I'm not really sure how to reproduce this issue, Could you possibly provide the exact steps you followed, starting from a clean install up to encountering the problem? Any additional insights would be greatly appreciated!

Have you tried it with multi culture ? I have the same issue with a fresh new project umbraco 13.2.2 (having multi culture). where I use this:

"Error404Collection": [
          {
            "Culture": "default",
            "ContentXPath": "$current/ancestor-or-self::home/descendant-or-self::page404"
          }
        ],

The same config still worked in other umbraco project 12.2.0 which has multi culture. I liked this xpth setting because it is not depended on node name (page404 is the error page document type alias)

nul800sebastiaan commented 6 months ago

@anas1987 Hmm, somehow that ($current/ancestor-or-self::home/descendant-or-self::page404) doesn't work for me in 12.2.0 either.

Breaking it down:

Your query can be greatly simplified by saying: //page404 - it does the same thing, go all the way up the tree, then find the first descendant page with document type alias page404. This works in a multi lingual setup as well.

If that doesn't work for you, as before, we'd need a way to reproduce.

anas1987 commented 6 months ago

@anas1987 Hmm, somehow that ($current/ancestor-or-self::home/descendant-or-self::page404) doesn't work for me in 12.2.0 either.

Breaking it down:

  • From the $current page (any page we're on in the website)
  • Go up to the level where a document type alias with home is found - we are now at the home page

    • or take the current page if that is home - we are now at the home page
  • When on the home page go down to all the descendants in the tree until you found a page with document type alias page404

    • or take the current page - note: this is -or-self unnecessary, we already know we are on home from the previous part)

Your query can be greatly simplified by saying: //page404 - it does the same thing, go all the way up the tree, then find the first descendant page with document type alias page404. This works in a multi lingual setup as well.

If that doesn't work for you, as before, we'd need a way to reproduce.

Thank for answer Sebastiaan, The different between $current/ancestor-or-self::home/descendant-or-self::page404 and //page404 that the long one ensure to come to the error page in the same culture, while the short one navigate to the first culture has page satisfy alias page404. Since I have different culture with different language so it is necessary to meet the right not found page with right language which is created under home page in each culture

nul800sebastiaan commented 6 months ago

So your tree is something like this?

anas1987 commented 6 months ago

So your tree is something like this?

  • home (en)

    • page404
  • home (de)

    • page404
  • home (se)

    • page404

Correct

anas1987 commented 6 months ago

The good news is: "Error404Collection": [ { "Culture": "default", "ContentXPath": "$site/page404" } ], works in both 13.2.2 and 12.2.0 and it meets not found page in current culture

nul800sebastiaan commented 6 months ago

@anas1987 That was going to be my next suggestion! πŸ˜‰ Much, much easier to read as well if someone ever takes over that site! πŸ‘

hfloyd commented 6 months ago

For anyone who wants to be able to support multiple-site-specific 404 Content nodes (as @anas1987 describes) and can't wait for @nul800sebastiaan's fix to be merged/released, or also wants to future-proof beyond XPath... I used the documentation example provided by @alina-tincas and created a version which supports the multi-site setup. Take a look here: Umbraco v13+ 404 Error IContentLastChanceFinder - Supporting multiple sites at root

Zeegaan commented 6 months ago

Fixed in https://github.com/umbraco/Umbraco-CMS/pull/16121 πŸš€