wanze / TemplateEngineFactory

🏭Provides ProcessWire integration for various template engines such as Twig.
MIT License
26 stars 11 forks source link

Wire404Exception not redirecting to 404 page #10

Closed gmclelland closed 5 years ago

gmclelland commented 6 years ago

On my calendar listing page I have the following code. It prevents the user from accessing a page that doesn't exist. For example: I have a calendar listing page that lists and paginates all the calendar postings on my site.

If my calendar listing page's pagination links only go to page 4, then I don't want search engines trying to index page 5. If someone tries to go to page 5, I want to show them a 404 not found page.

$paginated_calendar_postings is a variable that holds a $pages->find query to get the latest calendar postings.

// Don't let search engines index non-existing pages
// see https://processwire.com/blog/posts/processwire-2.6.18-updates-pagination-and-seo/
if(!count($paginated_calendar_postings) && $input->pageNum > 1) {
  throw new Wire404Exception();
}

To get it to work, I have to use:

// Don't let search engines index non-existing pages
// see https://processwire.com/blog/posts/processwire-2.6.18-updates-pagination-and-seo/
if(!count($paginated_calendar_postings) && $input->pageNum > 1) {
  $session->redirect($pages->get($config->http404PageID)->httpurl());
  throw new Wire404Exception();
}

Notice the $session->redirect?

I'm using the latest version of TemplateEngineFactory and TemplateEngineTwig.

In this case my controller is site/templates/calendar-list.php and my view is site/templates/calendar-list.twig

Let me know if you have any questions.

flydev-fr commented 6 years ago

@gmclelland I remember we already discussed this issue on the forum and it seem to be fixed with this commit.

Be sure that your module version is 1.1.3 and you could check with Tracy if the hook hookPageNotFound is triggered.

I tested to throw a 404 on a new app and it worked. The only issue I spotted is that the global file template is not included.

ps: I am using TemplateEngineProcessWire

gmclelland commented 6 years ago

Sorry @flydev-fr , I must have missed your response at https://processwire.com/talk/topic/19-how-do-i-make-processwire-display-the-404-page-not-found-page/?do=findComment&comment=159483.

Are you saying that you think this is core Processwire bug because the 404 from a bootstrapped script didn't work either? Did you happen to file an issue report by any chance?

Be sure that your module version is 1.1.3

I just checked again, and I'm definitely running TemplateEngineFactory 1.1.3 and TemplateEngineTwig 1.10.

you could check with Tracy if the hook hookPageNotFound is triggered.

I'm not sure how I can tell if the hookPageNotFound is triggered, but I did find this: hooks

Does that help? I removed the session redirect and that screenshot shows the Tracy Debugger when I visit a page that doesn't exist like page 5. In this case the pagination only goes to page 4. If I visit page 5 it shows me a page with header, title, footer, but no body content. Instead, it should show me a 404 page.

wanze commented 6 years ago

@gmclelland

Can you check the HTTP status code when you throw the 404Exception? If it is 404, then everything is correct except that the page object in your twig template does not correspond to the 404 page. This problem should be fixed with the latest release, but there may be other factors preventing it in your case, so we have to debug it further.

// Don't let search engines index non-existing pages
// see https://processwire.com/blog/posts/processwire-2.6.18-updates-pagination-and-seo/
if(!count($paginated_calendar_postings) && $input->pageNum > 1) {
  $session->redirect($pages->get($config->http404PageID)->httpurl());
  throw new Wire404Exception();
}

In this snippet, the 404 is never thrown as you are performing a redirect before. I have the feeling that this is not correct from a SEO point of view, the 404 should be displayed under your current page's url.

Cheers

gmclelland commented 6 years ago

In this snippet, the 404 is never thrown as you are performing a redirect before. I have the feeling that this is not correct from a SEO point of view, the 404 should be displayed under your current page's url.

When visiting a page that doesn't exist like www.mydomain.com/calendar/page9?q=music, if I remove the $session->.... line, then it displays my calendar postings page and not the 404 page. In this case the calendar postings page only shows a title and is missing the main content on the page.

If I view the Networking tab in Chrome Console, I see that the page is showing a 404 page not found when trying to access www.mydomain.com/calendar/page9?q=music. Maybe it is working? I just expected it to show the 404 page template when viewing that url that isn't actually valid. There is no page 9 for that search result.

FYI.. I'm running the latest TemplateEngineFactory and TemplateEngineTwig.

wanze commented 6 years ago

If I view the Networking tab in Chrome Console, I see that the page is showing a 404 page not found when trying to access www.mydomain.com/calendar/page9?q=music. Maybe it is working? I just expected it to show the 404 page template when viewing that url that isn't actually valid. There is no page 9 for that search result.

Thanks for your information. This means that the 404 is technically working correctly, but somehow Twig does not render the 404 page. If you can debug on your side, the following hook should forward the 404 page to your template engine: https://github.com/wanze/TemplateEngineFactory/blob/master/TemplateEngineFactory.module.php#L132-L143

I will run some more tests here. If I cannot reproduce it, would it be possible to send me the relevant controller and twig template?

wanze commented 5 years ago

This issue should not exist in the new 2.x major version of the module, since it now properly hooks into the page rendering process. We also have a test case for it. Please try to update to the new version.

Cheers

elabx commented 4 years ago

Hi! I know this shouldn't happen anymore but I am getting the 500 form an uncaught exception whenever i throw Wire404Exception within the controller. I am using TemplateEngineFactory 2.0 and the latest Smarty engine version.

wanze commented 3 years ago

Hi @elabx

Can you open a new issue and paste your controller code? I have no idea why ProcessWire would not catch a Wire404Exception - I guess we would need to debug this properly, e.g. with xdebug.