WordPress.org

Make WordPress Core

Opened 5 weeks ago

Last modified 4 weeks ago

#45958 new defect (bug)

Errors displayed on shutdown handler

Reported by: spacedmonkey Owned by:
Milestone: Awaiting Review Priority: high
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-screenshots has-patch needs-testing
Focuses: multisite Cc:

Description

PHP errors are displayed on shutdown handler screen for REST API requests.

See attached screenshot.

Attachments (7)

Screenshot 2019-01-12 at 13.55.17.png (681.9 KB) - added by spacedmonkey 5 weeks ago.
Screenshot 2019-01-12 at 14.04.30.png (671.3 KB) - added by spacedmonkey 5 weeks ago.
This also happens on RSS feeds
Screenshot 2019-01-12 at 14.18.50.png (685.0 KB) - added by spacedmonkey 5 weeks ago.
All appears on wp-trackback.php
Screenshot 2019-01-12 at 14.12.39.png (674.0 KB) - added by spacedmonkey 5 weeks ago.
All appears on wp-link-opml.php
Screenshot 2019-01-12 at 14.23.49.png (684.4 KB) - added by spacedmonkey 5 weeks ago.
Also appears on wp-signup.php
Screenshot 2019-01-13 at 00.47.50.png (175.8 KB) - added by spacedmonkey 5 weeks ago.
45958.diff (715 bytes) - added by spacedmonkey 5 weeks ago.

Change History (19)

#1 @spacedmonkey
5 weeks ago

  • Keywords servehappy added

@spacedmonkey
5 weeks ago

This also happens on RSS feeds

@spacedmonkey
5 weeks ago

All appears on wp-trackback.php

@spacedmonkey
5 weeks ago

All appears on wp-link-opml.php

@spacedmonkey
5 weeks ago

Also appears on wp-signup.php

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


5 weeks ago

#3 @afragen
5 weeks ago

Isn't this because WP_DEBUG and WP_DEBUG_DISPLAY are both true?

#4 @spacedmonkey
5 weeks ago

Nope, there are not setup like this. Screenshot below.

#5 @spacedmonkey
5 weeks ago

The issue is related to this line, that only hide errors on some conditions. Above is a list of conditions that public facing but are not XMLRPC_REQUEST, REST_REQUEST, WP_INSTALLING, Admin ajax or a JSON request.

#6 @afragen
5 weeks ago

Thanks for the explanation.

@spacedmonkey
5 weeks ago

#7 @spacedmonkey
5 weeks ago

  • Keywords has-patch needs-testing added; needs-patch removed

45958.diff first patch.

Simple change, just don't display error message for all requests.

#8 @schlessera
5 weeks ago

I think the patch breaks the WP_DISPLAY_DEBUG constant, as it is set early, and you're now overriding it no matter what. This line is meant to represent the exceptions for WP_DISPLAY_DEBUG is actually set to true. In such a case, the filtered requests you saw in the changed line here still need to not display anything to avoid breaking the response.

I had already looked into this as well, and the main problem is that the shutdown handler should decide whether or not to display the error, but at the moment the shutdown handler receives control, PHP has already printed the error.

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


5 weeks ago

#10 @pento
4 weeks ago

  • Milestone changed from Awaiting Review to 5.1

Adding to the 5.1 milestone for tracking.

#11 @flixos90
4 weeks ago

  • Milestone changed from 5.1 to Awaiting Review

This issue is not related to any changes made by or for Servehappy, therefore it's not relevant for the 5.1 milestone. Even without having the new PHP error template, that message would have been printed out if the server is configured like that.

The problem here is, as pointed out before, that WordPress does not actually disable displaying errors, even when WP_DEBUG_DISPLAY is not true. It only does so for some request types, for example AJAX requests. As far as I'm aware the issue has been raised before, but not been considered valid, as the server configuration should be responsible for that. I'm personally open to revisiting this, but it's not related to Servehappy, therefore I'll put it back to Awaiting Review.

#12 @pento
4 weeks ago

  • Keywords servehappy removed
  • Version trunk deleted
Note: See TracTickets for help on using tickets.