Skip to content

Ensure sites are always stopped - #514

Merged
fluiddot merged 2 commits into
trunkfrom
fix/force-server-stop
Sep 9, 2024
Merged

Ensure sites are always stopped#514
fluiddot merged 2 commits into
trunkfrom
fix/force-server-stop

Conversation

@fluiddot

@fluiddot fluiddot commented Sep 5, 2024

Copy link
Copy Markdown
Contributor

Related to https://github.com//issues/9018-gh-Automattic/dotcom-forge.

Proposed Changes

  • Ensure the server process is killed independently of the result of the stop message sent to the process. Ideally, we kill the process when the server stops gracefully, but we'll force this if the set timeout is reached.
  • Change the timeout of the server stop message from 2 minutes to 5 seconds. The 2-minute timeout is the default value we use for all server messages. My rationale for this change is that this value seems reasonable for starting the site or requests, but a server should be stopped sooner.
  • Catch any errors produced during the server stop. We'll log the error internally but there won't be any notification to the user. I thought to report the error to Sentry, but since it won't have any context due to being a timeout, I felt it would end up generating noise instead of being helpful.

Testing Instructions

  • Run the app with the command npm start.
  • Create a site.
  • Open the site's folder.
  • Create a file with the name test.php at root with the following content. It will make the request to run indefinitely.
<?php
while (true) {
    sleep(1);
}
  • Start the site.
  • Navigate to http://localhost:<PORT>/test.php.
  • Observe that the page gets in an indefinite loading state.
  • Stop the site.
  • Observe the site is stopped after 5 seconds.
  • Observe an error message is logged to the console.
Before (timeout has been set to 5 seconds for testing) After
studio-server-stop-before.mp4
studio-stop-site-after.mp4

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@fluiddot
fluiddot requested a review from a team September 5, 2024 16:41
@fluiddot fluiddot self-assigned this Sep 5, 2024
@fluiddot fluiddot changed the title Fix/force server stop Sep 5, 2024

@kozer kozer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @fluiddot ! It works as expected!

@fluiddot
fluiddot merged commit 30825b9 into trunk Sep 9, 2024
@fluiddot
fluiddot deleted the fix/force-server-stop branch September 9, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants