-
Notifications
You must be signed in to change notification settings - Fork 270
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
Gutenberg PR Preview: Broken check for the build artifact availability #1568
Comments
@brandonpayton I could use your help with this one because it looks like a server issue. ProductionI get a 200 and an empty LocallyStart a server:
I get a 400 error as expected. |
@bgrgicak are you setting GITHUB_TOKEN locally? |
The problem here is that the
Whereas the This diff seems to solve it: diff --git a/packages/playground/website/public/plugin-proxy.php b/packages/playground/website/public/plugin-proxy.php
index 59f9522afb..b1a55216a4 100644
--- a/packages/playground/website/public/plugin-proxy.php
+++ b/packages/playground/website/public/plugin-proxy.php
@@ -87,16 +87,6 @@ class PluginDownloader
continue;
}
- /*
- * Short-circuit with HTTP 200 OK when we only want to
- * verify whether the CI artifact seems to exist but we
- * don't want to download it yet.
- */
- if (array_key_exists('verify_only', $_GET)) {
- header('HTTP/1.1 200 OK');
- return;
- }
-
$allowed_headers = array(
'content-length',
'content-disposition',
@@ -112,6 +102,16 @@ class PluginDownloader
ob_end_flush();
flush();
+ /*
+ * Short-circuit with HTTP 200 OK when we only want to
+ * verify whether the CI artifact seems to exist but we
+ * don't want to download it yet.
+ */
+ if (array_key_exists('verify_only', $_GET)) {
+ header('HTTP/1.1 200 OK');
+ return;
+ }
+
// The API endpoint returns the actual artifact URL as a 302 Location header.
foreach ($artifact_res['headers'] as $header_line) {
$header_name = strtolower(substr($header_line, 0, strpos($header_line, ':'))); Any takes to make it into a PR? |
The endpoint for checking the zipped build availability returns a different payload depending on the artifact availability, but it's always a
200
:{ "error": "Request failed" }
The code that validates the artifact availability checks only whether it's a
200
or not, meaning it always passes that check. The consequence is that a wrong error is thrown, which can be misleading:The correct error thrown should be:
The text was updated successfully, but these errors were encountered: