WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 years ago

#18453 closed enhancement

Improve doing_it_wrong() and stop it from breaking WP — at Version 15

Reported by: azaozz Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: Cc:

Description (last modified by azaozz)

The output of _doing_it_wrong() when WP_DEBUG_DISPLAY is true is somehow not that useful. Perhaps we can do a backtrace there. This would also help beginner PHP programmers a lot.

Edit: storing the warnings in a global when WP_DEBUG_DISPLAY is true and outputting them at the top of the screen is a worthy enhancement too.

Change History (18)

@azaozz4 years ago

comment:1 @azaozz4 years ago

This is on the borderline for "plugin material" :) however being a plugin would mean the people that need it most won't probably have it.

comment:2 @nacin4 years ago

I would rather point them toward Log Deprecated Notices or Debug Bar, which handles these kinds of backtraces very effectively.

In my experience coding Log Deprecated Notices, different uses in core require different steps up the tree. It's painful, and exacting, and I believe it belongs in a plugin if only because it can be iterated quickly:

http://plugins.trac.wordpress.org/browser/log-deprecated-notices/trunk/log-deprecated-notices.php?rev=392669#L252

The best thing we can do for beginner programmers is point them toward established development tools.

comment:3 @scribu4 years ago

Agree with nacin. Maybe add a link to some codex page that would then point to the Debug Bar and Log Deprecated Notices plugins.

comment:4 @scribu4 years ago

  • Keywords close added

Marked #18512 as dup.

comment:5 follow-up: @nacin4 years ago

Reading #18512 and the patch from Chris, I think I still agree this is plugin material for now. How expensive is a backtrace? I don't think I'd like this running on every pageload, even with WP_DEBUG.

I'm tempted to include a link to the Debug Bar plugin in wp-config-sample.php where we reference WP_DEBUG, and maybe even in every trigger_error() in the _deprecated_* and _doing_it_wrong() functions. Or a link to codex.wordpress.org/Debugging_in_WordPress which can pimp things like WP_DEBUG, the Debug Bar, and Log Deprecated Notices.

Other than performance, the other issue is that the backtrace is going to lie, pretty often, depending on how it is used in the plugin and how the notice is triggered in core.As pointed about above, the code from LDN is pretty painful.

I'd be happy to revisit this but I'm just not convinced yet.

comment:6 in reply to: ↑ 5 ; follow-up: @mikeschinkel4 years ago

Replying to nacin:

Reading #18512 and the patch from Chris, I think I still agree this is plugin material for now. How expensive is a backtrace? I don't think I'd like this running on every pageload, even with WP_DEBUG..

Maybe I'm missing something, but doesn't this only run on a warning/error and not on every page load? If yes, the being expensive isn't an issue, right?

comment:7 in reply to: ↑ 6 ; follow-up: @nacin4 years ago

Replying to mikeschinkel:

Maybe I'm missing something, but doesn't this only run on a warning/error and not on every page load? If yes, the being expensive isn't an issue, right?

Every page load that has the warning/error -- which, in the case of conditional tags or enqueued scripts (the main two instances of _doing_it_wrong) would be pretty much every pageload. Once nice thing about our deprecated logging is that they don't have any expensive aspects to them, which means WP_DEBUG could be safely run in production.

I use Xdebug and I get my backtraces just fine ;-)

comment:8 in reply to: ↑ 7 @mikeschinkel4 years ago

Replying to nacin:

Every page load that has the warning/error -- which, in the case of conditional tags or enqueued scripts (the main two instances of _doing_it_wrong) would be pretty much every pageload.

You write code where there are warnings or errors on every page load? Seriously?

P.S. I think we need to give you a new nickname: "Dr. No." ;-)

comment:9 @azaozz4 years ago

Replying to nacin:

I'm tempted to include a link to the Debug Bar... Or a link to codex.wordpress.org/Debugging_in_WordPress which can pimp things like WP_DEBUG, the Debug Bar, and Log Deprecated Notices.

Yeah, perhaps this is the better option. Even if backtrace is not very expensive it's just another function to run. Including links to the plugins in all deprecated and _doing_it_wrong warnings would potentially be more beneficial too.

comment:10 follow-up: @scribu4 years ago

Including links to the plugins in all deprecated and _doing_it_wrong warnings would potentially be more beneficial too.

I hope we are thinking of doing that automatically from _doing_it_wrong() and _deprecated_*(), as opposed to duplicating it on each call to them.

comment:11 in reply to: ↑ 10 @azaozz4 years ago

Replying to scribu:

Of course, the links would be the same in all cases. No need to repeat them.

comment:12 @chrisbliss184 years ago

Here's some food for thought regarding performance and memory consumption impact due to backtrace parsing as found in my supplied patch.

I set up a site that had large objects running inside of nested actions and function calls, resulting in large backtrace outuput. I then kept track of the difference in memory consumption and execution time. The backtrace code is slightly modified from my supplied patch as it now can show either the source file and line number or class and function name (depending on the content of the specific backtrace entry). I'll update my patch to reflect this conditional shortly.

I ran three tests: Shallow, a single backtrace from the top of a theme's functions.php file (serialized backtrace size: 967 bytes). Single (Deep), a single backtrace from deep inside a large object with a very deep call stack (serialized backtrace size: 78,792 bytes). Multiple (Deep), a dozen backtraces deep inside call stacks of large objects (serialized backtrace size: 158,015 - 162,008 bytes). With these tests, I hoped to derive some expectations of the cost of such backtrace detail generation.

TypeSamplesSerialized Backtrace SizeTime DifferenceMemory Difference
Control103000
Shallow104967 bytes+0.0010389 seconds+20,736 bytes
Single (Deep)10478,793 bytes+0.0004531 seconds+388,303 bytes
Multiple (Deep)103158,015 bytes+0.0186892 seconds+913,524 bytes

Note that I used "memory_get_peak_usage()" to generate memory usage and "microtime( true )" to generate the timings. To keep the testing consistent in how it taxed the system, I used ab (Apache Bench) to run 100 tests (for a good sample size) with a concurrency of 1 (to make sure that the concurrency didn't open up some kind of PHP caching technique which showed when I upped the concurrency).

All in all, I think the test went well. The two deep samples aren't exactly cheap but they aren't too expensive either as I've seen very simple plugins make a bigger performance and memory impact than any of these test cases. Of course, since it is possible for there to be backtraces in worse scenarios than my samples, I suppose the value is purely academic.

I still would like to see if it is possible to incorporate simple backtrace output into the notice as this keeps the information inline with the kind offered by PHP's own notices. However, providing information on how to gain access to this information is still a positive step forward as I just don't like the idea of leaving novice developers in the dark on how to find the source of the notice. In other words, I don't want WordPress to have any dastardly "Null Pointer Exception" messages as found in Java where, if you see the message, you might as well just do something else as you aren't likely to find the source of the issue for a while in a large project. I would clarify what I mean more, but I'm sure that you'll figure it out. ;)

@chrisbliss184 years ago

Add simple backtrace output of just the file and line number of the offending code

@azaozz3 years ago

comment:13 @azaozz3 years ago

18453-2.patch drops the backtrace parsing and implements links to two plugins that can be used to get more info.

It also stops showing of the errors directly when WP_DEBUG_DISPLAY is true so we don't make WP unusable, "headers already sent...", etc. and implements a global that holds the error messages for displaying later at the top of the screen.

Last edited 3 years ago by azaozz (previous) (diff)

comment:14 @azaozz3 years ago

The whole doing_it_wrong concept is actually NUX for new developers and beginner PHP programmers that are starting to work with WordPress. Currently the errors it spits are very unfriendly and often break the rest of the output (headers already sent, etc.).

IMHO we can afford to make the experience a little better. We won't need to do a lot of hand-holding but can at least tell these users that they need a debugging plugin in a nicer way without breaking WP :) The same is true for all the deprecated notices.

In that terms I think something like 18453-2.patch is a needed enhancement and it should include all deprecated notices too. The only thing it still needs is to remove the direct links to plugins and instead link to a codex page about debugging in WordPress.

Last edited 3 years ago by azaozz (previous) (diff)

comment:15 @azaozz3 years ago

  • Description modified (diff)
  • Keywords close removed
  • Milestone changed from Future Release to 3.3
  • Summary changed from Improved doing_it_wrong() to Improve doing_it_wrong() and stop it from breaking WP

Didn't realize that at first but storing the warnings in a global when WP_DEBUG_DISPLAY is true and outputting them at the top of the screen prevents breaking of all XHR requests and wp_cron. It also will prevent these warning from showing in search engines if WP_DEBUG is run on a live site.

Thinking we should handle all deprecated and doing_it_wrong warning as in 18453-2.patch.

Note: See TracTickets for help on using tickets.