Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#21533 closed defect (bug) (fixed)

Error suppression in wp-db is lame: Part 2

Reported by: Viper007Bond Owned by:
Priority: normal Milestone: 3.5
Component: Warnings/Notices Version: 3.4.1
Severity: normal Keywords: has-patch needs-unit-tests
Cc: scribu, kpayne@…, dkikizas@…

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 9 months ago.
21533-unit-test.patch (1.9 KB) - added by kurtpayne 9 months ago.
21533-unit-test.2.patch (2.6 KB) - added by kurtpayne 9 months ago.
Updated with test for get too

Download all attachments as: .zip

Change History (16)

  • 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.

  • Cc scribu added

comment:3 follow-up: ↓ 5   westi9 months ago

From memory what this broke was HyperDB on WP.org

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

comment:5 in reply to: ↑ 3 ; follow-up: ↓ 6   Viper007Bond9 months 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...

comment:6 in reply to: ↑ 5   SergeyBiryukov9 months ago

Replying to Viper007Bond:

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

I guess westi was referring to [16320].

Updated with test for get too

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

  • Cc dkikizas@… added

Closed #21574 as a duplicate.

comment:10 follow-up: ↓ 11   scribu9 months 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

comment:11 in reply to: ↑ 10   Viper007Bond9 months 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. ;)

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

In [21513]:

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

Note: See TracTickets for help on using tickets.