Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#25788 closed enhancement (fixed)

Increase timeout for `plugins/update-check` API request

Reported by: johnbillion's profile johnbillion Owned by: johnbillion's profile johnbillion
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch commit
Focuses: Cc:

Description

On my various local test installs I have a single directory rammed full of plugins that is shared between installs. This directory often has over 100 plugins in it, and this often causes the plugin update check to time out (due to the number of plugins that are being checked I presume).

We should increase the timeout for the api.wordpress.org/plugins/update-check requests. We could increase it based on the number of plugins being checked. For example, count( $plugins ) / 10 might be a good timeout, meaning an increase of one second for every ten plugins on the site (with the current three seconds as a minimum).

Thoughts?

Attachments (1)

25788.diff (1.3 KB) - added by dd32 11 years ago.

Download all attachments as: .zip

Change History (15)

#2 @dd32
11 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.8

Basing the timeout on the number of plugins seems like a decent idea to me.

Not completing the request during a pageload is incredibly annoying (as you know there's an update, but it's timing out) and if an extra second can be added for a large number of plugins (for a page that is already slow from that number of plugins) then it seems worth it.

On the other hand, this does mean an extra second (or two) until the request times out in the event the server has poor connectivity.

Moving to 3.8 pending discussion

@dd32
11 years ago

#3 @dd32
11 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

Still on the fence, but can't do any harm. Any objections?

#4 @dd32
11 years ago

  • Milestone changed from 3.8 to Future Release

#5 @SergeyBiryukov
11 years ago

#27091 was marked as a duplicate.

#6 @johnbillion
10 years ago

  • Keywords commit added; 2nd-opinion removed
  • Milestone changed from Future Release to 4.0

#7 @johnbillion
10 years ago

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

In 28782:

Increase the timeout for plugin and theme update checks based on how many are being checked. Props dd32. Fixes #25788

#8 @kovshenin
10 years ago

Re r28782: not a big deal, but it seems like (int) $v is ~ 200 times faster than intval( $v ), especially if you're not expecting strings and you're working in base 10.

#10 @SergeyBiryukov
10 years ago

In 28823:

Use direct typecasting instead of intval() for better performance.

props kovshenin.
see #25788.

#11 @Denis-de-Bernardy
10 years ago

Fwiw, this is still timing out for me. Due to a slow connection, I imagine, since I've only 10 plugins on my localhost install.

#12 follow-up: @SergeyBiryukov
10 years ago

#20002 would probably resolve this completely.

#13 in reply to: ↑ 12 @Denis-de-Bernardy
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to SergeyBiryukov:

#20002 would probably resolve this completely.

Doubtful. The problem occurs at lines such as these:

Why we're triggering an E_USER_* error on this is beyond me, btw. This fires up the Symfony debugger, and leaves you staring at a screen full with the details of the exception.

Methinks the correct fix would be to get rid of that, and trigger the error only when the cron is running for the sake of logging it. On the front end, it should instead display an appropriately colored (warning) message in the admin UI and trigger no PHP error at all.

Moreover, and as stated in a separate ticket, I can't hep but point out why anyone privileged with an SSL-enabled HTTP transport gets to see this kind of warning but not those who don't.

Lastly, I'd like to highlight that the timeout is either ignored or not honored, or the wordpress.org API times out earlier than whatever is specified. Because, well, I increased it to a ridiculously high value (30 seconds) for 10 plugins, and it's still timing out -- long before thirty seconds elapse.

#14 @johnbillion
10 years ago

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

Denis, these are valid points (and I agree on the unwanted E_USER_* errors) but let's open a new ticket for them as the particular issue in this ticket was fixed.

Note: See TracTickets for help on using tickets.