Make WordPress Core

Opened 16 years ago

Closed 9 years ago

Last modified 3 days ago

#13657 closed enhancement (fixed)

When WP_DEBUG is true show better errors for establishing database connection

Reported by: sivel Owned by: SergeyBiryukov
Priority: low Milestone: 5.1
Component: Database Version:
Severity: trivial Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

Right now no matter if WP_DEBUG is enabled or not we always show some generic errors for Error establishing a database connection

We should display the specific error when WP_DEBUG is enabled.

Attachments (3)

13657.diff​ (2.6 KB ) - added by sivel 16 years ago.
13657.2.diff​ (695 bytes ) - added by Mte90 10 years ago.
check for WB_DEBUG and WP_DEBUG_DISPLAY
13657.3.diff​ (450 bytes ) - added by Mte90 9 years ago.
new patch with in bail method

Download all attachments as: .zip

Change History (20)

#1 @sivel
16 years ago

Patch is just an idea of how we might do this. Open to other suggestions and patches.

@sivel
16 years ago

#2 @nacin
16 years ago

I would tend to think we should not be exposing the output of mysql_error() on any site. Or, if we do, then we at least also check for display_errors/WP_DEBUG_DISPLAY, in case they want WP_DEBUG (as can be common in production) but are logging it.

#3 @nacin
16 years ago

  • Keywords early removed

#4 @nacin
16 years ago

  • Milestone Awaiting Triage → Future Release

#5 @sivel
16 years ago

The way that translations are handled for these errors makes it difficult to do conditionals like this. Mainly due to duplicating large portions of code and strings. Well maybe not large but more than I am comfortable with duplicating.

I may just need to stare at the code for a bit to get a creative solution, that is unless someone has recommendations.

#6 @mbijon
14 years ago

  • Cc mike@… added

#7 @ryan
12 years ago

  • Owner ryan removed
  • Status new → assigned

#8 @chriscct7
11 years ago

  • Keywords dev-feedback added

#9 @pento
11 years ago

  • Keywords needs-refresh added; dev-feedback removed

The strings have since been split up, making it much easier to insert a conditional string when WP_DEBUG and WP_DEBUG_DISPLAY are defined.

@Mte90
10 years ago

check for WB_DEBUG and WP_DEBUG_DISPLAY

#10 @Mte90
10 years ago

  • Keywords dev-feedback added; needs-refresh removed

I updated the patch but I am not sure about mysql_error because maybe wpdb have a wrapper for that but I am not an expert for this part.

#11 @pento
9 years ago

  • Keywords needs-refresh added; dev-feedback removed

It would be best to put this in wpdb::bail(). That method will always be the one called when WPDB needs to kill page loading. There's no need to check WP_DEBUG && WP_DEBUG_DISPLAY here, as wpdb::$show_errors is set to that.

I would suggest appending it to $message at the start. Also, don't forget to check mysqli_error(). :-)

@Mte90
9 years ago

new patch with in bail method

This ticket was mentioned in ​Slack in #core by mte90. ​View the logs.


9 years ago

#13 @SergeyBiryukov
9 years ago

  • Milestone Future Release → 5.0

#14 @SergeyBiryukov
9 years ago

  • Owner set to SergeyBiryukov
  • Resolution → fixed
  • Status assigned → closed

In 42716:

Database: If both WP_DEBUG and WP_DEBUG_DISPLAY are true, display a more specific error message in wpdb::bail().

Props sivel, Mte90, SergeyBiryukov.
Fixes #13657.

#15 @SergeyBiryukov
8 years ago

  • Keywords needs-refresh removed
  • Milestone 5.0 → 5.1

This ticket was mentioned in ​PR #14081 on ​WordPress/wordpress-develop by ​@therssoftware.


3 days ago
#16

  • Keywords has-unit-tests added

Building on @swissspidy's wp_safe_remote_request_multiple() in PR #13657, I have implemented download_url_multiple() and wired it into Plugin_Upgrader::bulk_upgrade() and Theme_Upgrader::bulk_upgrade().

Benchmark Results (5 Real WP.org Packages)

Tested downloading 5 real production plugins (hello-dolly, wp-multibyte-patch, classic-widgets, regenerate-thumbnails, duplicate-post):

  • Sequential (download_url): 6.315s
  • Concurrent (download_url_multiple): 2.147s
  • Speedup: 2.94x faster (66% latency reduction)

Implementation Summary

  1. download_url_multiple() added to src/wp-admin/includes/file.php with concurrent streaming.
  2. Extracted private _wp_handle_download_response() helper, removing code duplication between single and multiple downloads (shares Content-Disposition, MIME checks, MD5 verification, and .sig checks).
  3. Plugin_Upgrader & Theme_Upgrader pre-fetch compatible packages in parallel before the update loop; unconsumed files are cleaned up safely.
  4. Resilience: Automatic fallback to package URL if any single batch item fails.
  5. Full Tests: Added dedicated unit tests in downloadUrlMultiple.php (7 tests, 33 assertions) and integration tests in bulkUpgrader.php (3 tests, 14 assertions). 100% passing; 0 PHPCS warnings/errors.

Branch: feature/bulk-upgrader-concurrent-downloads
Full details and implementation notes: See attached PR description.

This ticket was mentioned in ​PR #14081 on ​WordPress/wordpress-develop by ​@therssoftware.


3 days ago
#17

Building on @swissspidy's wp_safe_remote_request_multiple() in PR #13657, I have implemented download_url_multiple() and wired it into Plugin_Upgrader::bulk_upgrade() and Theme_Upgrader::bulk_upgrade().

Benchmark Results (5 Real WP.org Packages)

Tested downloading 5 real production plugins (hello-dolly, wp-multibyte-patch, classic-widgets, regenerate-thumbnails, duplicate-post):

  • Sequential (download_url): 6.315s
  • Concurrent (download_url_multiple): 2.147s
  • Speedup: 2.94x faster (66% latency reduction)

Implementation Summary

  1. download_url_multiple() added to src/wp-admin/includes/file.php with concurrent streaming.
  2. Extracted private _wp_handle_download_response() helper, removing code duplication between single and multiple downloads (shares Content-Disposition, MIME checks, MD5 verification, and .sig checks).
  3. Plugin_Upgrader & Theme_Upgrader pre-fetch compatible packages in parallel before the update loop; unconsumed files are cleaned up safely.
  4. Resilience: Automatic fallback to package URL if any single batch item fails.
  5. Full Tests: Added dedicated unit tests in downloadUrlMultiple.php (7 tests, 33 assertions) and integration tests in bulkUpgrader.php (3 tests, 14 assertions). 100% passing; 0 PHPCS warnings/errors.

Branch: feature/bulk-upgrader-concurrent-downloads
Full details and implementation notes: See attached PR description.

Note: See TracTickets for help on using tickets.