WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 13 months ago

Last modified 13 months ago

#26262 closed defect (bug) (wontfix)

Ajax requests shouldn't display errors

Reported by: pento Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords:
Focuses: Cc:

Description

Similar to #23811, displaying PHP errors in an Ajax response will probably cause a critical error in the client code. Instead, we should hide errors.

This is particularly important as #21663 is unlikely to make WordPress 3.8, which means all folks using WP_DEBUG on PHP 5.5 will be getting warnings they can't do anything about.

Attachments (1)

26262.diff (387 bytes) - added by pento 21 months ago.

Download all attachments as: .zip

Change History (19)

@pento21 months ago

comment:1 @pento21 months ago

  • Keywords has-patch dev-feedback added

comment:3 follow-ups: @SergeyBiryukov21 months ago

I think this would make debugging AJAX issues harder. Code in production shouldn't have WP_DEBUG on.

comment:4 in reply to: ↑ 3 @pento21 months ago

Replying to SergeyBiryukov:

I think this would make debugging AJAX issues harder.

It will make doing anything with PHP 5.5 impossible, unless #21663 makes 3.8.

comment:5 @bjornjohansen21 months ago

What if we introduce a new constant (e.g. WP_DEBUG_LEVEL), and if defined, we use its value for error_reporting()?
Like this: http://pastebin.com/M5HU8y40

comment:6 follow-up: @dd3221 months ago

It will make doing anything with PHP 5.5 impossible, unless #21663 makes 3.8.

FWIW, Developing with WP_DEBUG + PHP 5.5 since 3.7 has been a pain.. I don't think it's that much of a problem for a savvy person to install the mysqli plugin or pdo plugin though.

comment:7 in reply to: ↑ 6 @bjornjohansen21 months ago

Replying to dd32:

FWIW, Developing with WP_DEBUG + PHP 5.5 since 3.7 has been a pain.. I don't think it's that much of a problem for a savvy person to install the mysqli plugin or pdo plugin though.

The MySQLi/PDO plugins aren't production ready, which means you'll have to maintain two separate codebases for dev and prod. Shouldn't be hard in this case, but still not a good principle.

There is also something wrong about installing a plugin to fix something which is outright wrong in core. It should be fixed in core.

A savvy person can also disable the error output and tail the error log instead. Or write their own error handler.

comment:8 @markoheijnen21 months ago

Both plugins are production ready. MySQLi more then the PDO one obviously.

Hiding error on AJAX requests is wrong to do. When you enable WP_DEBUG you would expect that things break when something goes wrong. And we shouldn't do it because of 1 issue that can be solved.

comment:9 @ocean9021 months ago

  • Milestone changed from 3.8 to Awaiting Review

comment:10 @nacin20 months ago

  • Component changed from General to Bootstrap/Load

comment:11 follow-up: @Justin Folvarcik17 months ago

I would suggest adding this information to the AJAX documentation pages. It could have saved me over five hours of trying to debug something was wasn't even a bug, and it's likely to confuse more people in the future because this information is not documented anywhere but here (that I could find).

comment:12 in reply to: ↑ 11 @adamsilverstein17 months ago

Replying to Justin Folvarcik:

I would suggest adding this information to the AJAX documentation pages. It could have saved me over five hours of trying to debug something was wasn't even a bug, and it's likely to confuse more people in the future because this information is not documented anywhere but here (that I could find).

Good suggestion Justin! I know how frustrating debugging these types of issues can be, only to realize hours later the problem was something out-of-sight you couldn't observe directly.

I think this issue (corrupted response due to warning messages) is covered on the codex ajax instructions in the debugging section here- http://codex.wordpress.org/AJAX_in_Plugins#Debugging.

It looks like this section could use some improvements based on your experience. Note that the codex is community driven, meaning you can help improve it! Once you are logged into wordpress.org, you should see an edit link that will allow you to make changes. I will keep my eye on the page to review what you add.

Thanks for raising the issue - improving the documentation may help the next person with a similar problem.

comment:13 in reply to: ↑ 3 ; follow-up: @SergeyBiryukov17 months ago

Replying to SergeyBiryukov:

I think this would make debugging AJAX issues harder. Code in production shouldn't have WP_DEBUG on.

On second thought, I saw some users with AJAX issues on support forums (Media Library not loading in the modal due to a broken plugin, for example). Looks like 26262.diff would have prevented that.

comment:14 in reply to: ↑ 13 @adamsilverstein17 months ago

Replying to SergeyBiryukov:

Replying to SergeyBiryukov:

I think this would make debugging AJAX issues harder. Code in production shouldn't have WP_DEBUG on.

On second thought, I saw some users with AJAX issues on support forums (Media Library not loading in the modal due to a broken plugin, for example). Looks like 26262.diff would have prevented that.

I agree with the first part: I would be hesitant to suppress this output as it will make debugging more difficult. The errors can actually be seen when viewing the response in a browser debugger. Plenty of instruction out there about how to turn of error messages.

comment:15 follow-up: @Justin Folvarcik17 months ago

I've updated the documentation to hopefully make it a bit clearer. I really hope it helps others avoid the fiasco that I went through.

comment:16 in reply to: ↑ 15 @adamsilverstein17 months ago

Replying to Justin Folvarcik:

I've updated the documentation to hopefully make it a bit clearer. I really hope it helps others avoid the fiasco that I went through.

Nice work, thank you for your contribution!

comment:17 @pento13 months ago

  • Keywords has-patch dev-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

This can also be worked around by using WP_DEBUG_LOG instead of sending the messages to the browser. There are sufficient alternatives that we shouldn't do anything about this in core.

comment:18 @johnbillion13 months ago

Gratuitous self-promotion: my Query Monitor plugin catches errors in AJAX requests, alerts you to them in the admin toolbar, and displays their details in the browser console. Prevents the sort of breakage mentioned in this ticket.

Note: See TracTickets for help on using tickets.