WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#21533 closed defect (bug) (fixed)

Error suppression in wp-db is lame: Part 2

Reported by: Viper007Bond Owned by:
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4.1
Component: Warnings/Notices Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

Previously: #15402

This was fixed in [16321] however that was built upon [16320] and both were reverted in [16336] because it "Breaks things". What things it breaks I have no idea though (bad westi for not using a better commit message ;)). However I assume it was [16320] that broke things and not [16321].

[16321] seems perfectly valid to me and we should re-apply it along with a fix to __get() that throws a warning when $result isn't set yet.

Attachments (3)

21533.patch (595 bytes) - added by Viper007Bond 6 years ago.
21533-unit-test.patch (1.9 KB) - added by kurtpayne 6 years ago.
21533-unit-test.2.patch (2.6 KB) - added by kurtpayne 6 years ago.
Updated with test for get too

Download all attachments as: .zip

Change History (16)

@Viper007Bond
6 years ago

#1 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 3.5

Since [21472], I get errors in Debug Bar:

WARNING: wp-includes/wp-db.php:1079 - mysql_free_result(): supplied argument is not a valid MySQL result resource

Moved to wpdb::flush(), mysql_free_result() now runs earlier than $this->result is set:
http://core.trac.wordpress.org/browser/trunk/wp-includes/wp-db.php?rev=21473#L1138

21533.patch fixes the warning.

#2 @scribu
6 years ago

  • Cc scribu added

#3 follow-up: @westi
6 years ago

From memory what this broke was HyperDB on WP.org

#4 @kurtpayne
6 years ago

  • Cc kpayne@… added
  • Keywords needs-unit-tests added

#5 in reply to: ↑ 3 ; follow-up: @Viper007Bond
6 years ago

Replying to westi:

From memory what this broke was HyperDB on WP.org

Checking if it was a resource was what broke it? Crazy...

#6 in reply to: ↑ 5 @SergeyBiryukov
6 years ago

Replying to Viper007Bond:

Checking if it was a resource was what broke it? Crazy...

I guess westi was referring to [16320].

@kurtpayne
6 years ago

Updated with test for get too

#7 @kurtpayne
6 years ago

Unit test added. It functions the same way as debug bar -- by spying on the error handler.

#8 @demetris
6 years ago

  • Cc dkikizas@… added

#9 @SergeyBiryukov
6 years ago

Closed #21574 as a duplicate.

#10 follow-up: @scribu
6 years ago

I now get this notice on every admin page in trunk:

WARNING: wp-includes/wp-db.php:1079 - mysql_free_result() expects parameter 1 to be resource, boolean given

#11 in reply to: ↑ 10 @Viper007Bond
6 years ago

Replying to scribu:

I now get this notice on every admin page in trunk:

Yep, this is why this ticket was opened. See above. ;)

#12 @nacin
6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

#13 @nacin
6 years ago

In [21513]:

@since for wpdb's result property. props SergeyBiryukov. see #21533.

Note: See TracTickets for help on using tickets.