Drupal seems to return a 200 OK HTTP status code for exception errors thrown by _drupal_exception_handler(). This doesn't seem right, since that causes the error page to be cached, for example in Varnish. Is there a reason for this behaviour? I couldn't find an issue for the bug, but it has been mentioned on the support forum at https://www.drupal.org/node/2598442.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ZeiP created an issue. See original summary.

ZeiP’s picture

Status: Active » Needs review
FileSize
772 bytes

Attached is a simple patch to return 500 Internal Server Error instead.

cilefen’s picture

Does Drupal 8 do the same thing?

iamEAP’s picture

Status: Needs review » Needs work

Ran into this as well. One common way you might hit this is if you have a cache class that throws an exception in the page cache bootstrap phase (e.g. Redis cannot connect or server goes away). This will cause the error logger to attempt to load the maintenance theme, which will attempt to load modules/themes, which will eventually result in a cache_get() attempt (which will throw another exception).

Moving back to needs work as you would want to set response code to 500 regardless of whether or not errors can be displayed.

iamEAP’s picture

Status: Needs work » Needs review
FileSize
777 bytes

Patch to account for that, with explanation text.

iamEAP’s picture

Updated patch with minor change to status text to better align with HTTP standards.

cilefen’s picture

cilefen’s picture

SylvainM’s picture

Status: Needs review » Reviewed & tested by the community

Tested and it works great, thanks!

tobybellwood’s picture

We've just deployed this patch into production for all of our sites (we're using docker with Redis pods, and they'll generate 500's on respawn which were being occasionally cached at the edge as 200's before this patch).

Given the relatively low complexity of the patch and large upside in these scenarios, we'd support this patch making it's way into core.

Pol’s picture

Issue tags: +Drupal 7.67 target

Hi all,

I would like to push this patch forward.
Do you know if it's already in D8?

Thanks.

Pol’s picture

I mostly sure that this is already in D8, see core/includes/errors.inc, around line 265.

Fabianx’s picture

Issue tags: +Pending Drupal 7 commit

RTBC + 1, let get this in -- it's a good stepping stone (incidentally I also ran into this yesterday)

  • Fabianx committed 117ef34 on 7.x authored by Pol
    Issue #2666908 by iamEAP, cilefen: HTTP status 200 returned for ”...
Pol’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit

Fixed! Thanks all !

Status: Fixed » Closed (fixed)

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