WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 3 months 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 months ago.
Screenshot 2019-01-12 at 14.04.30.png (671.3 KB) - added by spacedmonkey 5 months ago.
This also happens on RSS feeds
Screenshot 2019-01-12 at 14.18.50.png (685.0 KB) - added by spacedmonkey 5 months ago.
All appears on wp-trackback.php
Screenshot 2019-01-12 at 14.12.39.png (674.0 KB) - added by spacedmonkey 5 months ago.
All appears on wp-link-opml.php
Screenshot 2019-01-12 at 14.23.49.png (684.4 KB) - added by spacedmonkey 5 months ago.
Also appears on wp-signup.php
Screenshot 2019-01-13 at 00.47.50.png (175.8 KB) - added by spacedmonkey 5 months ago.
45958.diff (715 bytes) - added by spacedmonkey 5 months ago.

Change History (21)

#1 @spacedmonkey
5 months ago

  • Keywords servehappy added

@spacedmonkey
5 months ago

This also happens on RSS feeds

@spacedmonkey
5 months ago

All appears on wp-trackback.php

@spacedmonkey
5 months ago

All appears on wp-link-opml.php

@spacedmonkey
5 months ago

Also appears on wp-signup.php

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


5 months ago

#3 @afragen
5 months ago

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

#4 @spacedmonkey
5 months ago

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

#5 @spacedmonkey
5 months 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 months ago

Thanks for the explanation.

@spacedmonkey
5 months ago

#7 @spacedmonkey
5 months 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 months 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 months ago

#10 @pento
5 months ago

  • Milestone changed from Awaiting Review to 5.1

Adding to the 5.1 milestone for tracking.

#11 @flixos90
5 months 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
5 months ago

  • Keywords servehappy removed
  • Version trunk deleted

#13 @SergeyBiryukov
3 months ago

Is this still relevant after #45989?

#14 @spacedmonkey
3 months ago

@SergeyBiryukov still valid, it is about the errors showing at all. See the screenshots.

Note: See TracTickets for help on using tickets.