A small suggestion to replace the call to exit in robotstxt_robots() with drupal_page_footer() to allow Drupal to respect page caching.

See http://drupal.org/node/1791978 for details.

Patch attached.

CommentFileSizeAuthor
#5 robotstxt-drupal_exit.patch302 bytesles lim
robotstxt-respect-page-cache.patch309 bytesjohan.gant

Comments

hass’s picture

hass commented
Status: Needs review » Postponed (maintainer needs more info)

But the module does not cache the robots.txt as I know. The variable we are reading is cached, but there is no need to cache more as I know.

johan.gant’s picture

johan.gant commented

This is more to do with the request/response process than where the data for robots.txt is stored.

With a call to exit() you will always get a response header with the following: X-Cache:MISS

If you're behind an HTTP cache such as Varnish or have Drupal's anonymous page cache enabled you don't want this to happen as it could, in principle, add unwanted request load to your backend webserver(s). In this instance, if you used drupal_page_footer() instead of exit() your first request will get X-Cache:MISS and subsequent requests will be X-Cache:HIT... until the cache expires for that item at least.

drupal_page_footer() ensures final end of request tasks are correctly handled. A call to exit() is far less helpful than drupal_page_footer(). That's my point and what my patch aims to address.

les lim’s picture

les lim
he/they
Primary language English
commented
Status: Postponed (maintainer needs more info) » Needs review

Alternatively, there's drupal_exit() as well:

http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_exit/7

hass’s picture

hass commented
Status: Needs review » Needs work

So we go with drupal_exit().

les lim’s picture

les lim
he/they
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new robotstxt-drupal_exit.patch302 bytes

New patch.

hass’s picture

hass commented
Status: Needs review » Reviewed & tested by the community
hass’s picture

hass commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 9a933a7 on 7.x-1.x, 8.x-1.x authored by Les Lim, committed by hass: 
    Issue #1858178 by lesmana: Call to exit in callback function breaks page...
hass’s picture

hass commented
Issue summary: View changes
Parent issue: » #1858178: call to exit in callback function breaks page caching