Closed jeroenpf closed 1 month ago
For reference, here are the changes introduced in Playground: https://github.com/WordPress/wordpress-playground/compare/v0.9.39...v0.9.44
Looks good. Thanks for fixing this in the upstream @jeroenpf (https://github.com/WordPress/wordpress-playground/pull/1752) and upgrading Playground in Studio!
I've tested it more, but it seems that when I try to start the site configured to use MySQL, it crashes:
node:internal/errors:497
ErrorCaptureStackTrace(err);
^
TypeError [ERR_HTTP_INVALID_HEADER_VALUE]: Invalid value "undefined" for header "set-cookie"
at ServerResponse.setHeader (node:_http_outgoing:655:3)
at /Volumes/Sites/local-environment/.webpack/main/siteServerProcess.js:61899:21
at Generator.next (<anonymous>)
at fulfilled (/Volumes/Sites/local-environment/.webpack/main/siteServerProcess.js:61839:58)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
code: 'ERR_HTTP_INVALID_HEADER_VALUE'
}
Node.js v20.9.0
When I use Studio 1.1.2, it works fine.
Also, it seems that the above crash happens when I'm using MySQL, but with incorrect credentials configured in wp-config.php
. When I use the correct credentials, I see a broken site:
When I use Studio 1.1.2, both cases work: incorrect credentials show the page informing me about the problems connecting to MySQL, and correct credentials show the correct site.
I've tested it more, but it seems that when I try to start the site configured to use MySQL, it crashes:
node:internal/errors:497 ErrorCaptureStackTrace(err); ^ TypeError [ERR_HTTP_INVALID_HEADER_VALUE]: Invalid value "undefined" for header "set-cookie" at ServerResponse.setHeader (node:_http_outgoing:655:3) at /Volumes/Sites/local-environment/.webpack/main/siteServerProcess.js:61899:21 at Generator.next (<anonymous>) at fulfilled (/Volumes/Sites/local-environment/.webpack/main/siteServerProcess.js:61839:58) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) { code: 'ERR_HTTP_INVALID_HEADER_VALUE' } Node.js v20.9.0
When I use Studio 1.1.2, it works fine.
@wojtekn how did you reproduce it?
Based on the error message, I wonder if it's related to https://github.com/Automattic/studio/pull/532 🤔 .
@wojtekn how did you reproduce it?
Here are the steps:
wp-content/database/
, mu-plugins/sqlite-database-integration
and wp-content/db.php
.Expected behavior: the following page is served:
Current behavior: site crashes with set-cookie error
@wojtekn how did you reproduce it?
Here are the steps:
- Start Studio in dev mode from this branch
- Add a site and open it in IDE
- Remove
wp-content/database/
,mu-plugins/sqlite-database-integration
andwp-content/db.php
.- Open WP Admin
Expected behavior: the following page is served:
Current behavior: site crashes with set-cookie error
Ah, good catch. As I shared in https://github.com/Automattic/studio/pull/561#issuecomment-2376577307, this is a bug most likely introduced in https://github.com/Automattic/studio/pull/532. I've pushed a fix in https://github.com/Automattic/studio/pull/561/commits/0f0d45eb40f067be928bb952e290023fea868516.
Now when accessing WP-Admin in the scenario shared in https://github.com/Automattic/studio/pull/561#issuecomment-2376586404, it shows the error page:
I encountered a weird behavior when testing a site with MySQL files inside the site's folder, specifically when having the symlink mysql.sock
. I presume this is an edge case, as users won't have these files in the site's folder, however, this uncovers a bug related to not-resolvable symlinks and the site initialization. I'll open a separate GitHub issue as it's not related to this PR.
UPDATE: Here's the issue 9257-gh-Automattic/dotcom-forge.
Is this still good for a merge?
Is this still good for a merge?
Yes, from my side it's ready to be merged :shipit: .
I noticed that the patch introduced in https://github.com/Automattic/studio/pull/532 is no longer being applied is giving a warning due to version mismatch but is still being applied after upgrading the Playground dependencies. I'll open a PR to update it.
UPDATE: Here's the PR https://github.com/Automattic/studio/pull/580.
Proposed Changes
Testing Instructions
-
Pre-merge Checklist