Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin proxy: Stream the response #879

Closed
adamziel opened this issue Dec 18, 2023 · 3 comments
Closed

Plugin proxy: Stream the response #879

adamziel opened this issue Dec 18, 2023 · 3 comments

Comments

@adamziel
Copy link
Collaborator

plugin-proxy.php currently buffers the entire zip artifact downloaded from GitHub. This means the browser can only start downloading, say, a WordPress core build from a specific PR once the PHP script has the full response. This significantly slows down the WordPress PR previewer. Let's make sure the plugin-proxy.php streams the response body as it receives the bytes.

@dmsnell
Copy link
Member

dmsnell commented Dec 20, 2023

For some nginx configurations I had to add the following header to prevent it from buffering the PHP response.

X-Accel-Buffering: no

Note also that it's important to close any outstanding output buffers. This may break things if done with disregard, but here in the proxy we should have enough oversight to do it safely.

set_time_limit( 0 );
ob_implicit_flush( 1 );
while ( ob_end_flush() ) {
	continue;
}

@adamziel
Copy link
Collaborator Author

adamziel commented Dec 21, 2023

This was resolved in #889 which uses CURL CURLOPT_WRITEFUNCTION with flush(); for streaming. It seems to be working pretty well, but I wonder if adding this bit just to be safe would still be a good idea:

set_time_limit( 0 );
ob_implicit_flush( 1 );
while ( ob_end_flush() ) {
	continue;
}

Note that ob_start() is never called.

@dmsnell
Copy link
Member

dmsnell commented Dec 21, 2023

Note that ob_start() is never called.

In WordPress it's important to run this loop because a plugin may have called ob_start(), or the code is complicated enough that it may be hard to track down places it was called. This is safe to call even when ob_start() has never been called.

in #889 which uses CURL CURLOPT_WRITEFUNCTION with flush();

As long as it's working then it's good, but to point it out, my comment was about nginx which operates above the level of PHP. It's possible that in some environments the curl functionality will be buffered because of the frontend web server, which wraps the responses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants