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

Switching to Live branch: Prevent code from executing after new code has been put in place. #98

Merged

Conversation

bisko
Copy link
Contributor

@bisko bisko commented Mar 31, 2020

This PR is a result of Automattic/jetpack#15209 .

After switching to another branch that's not "current", compared to the "old" branch that we switch out of, can see warnings or errors in the error log.

This happens because we continue executing actions and shutdown handlers after the code on the filesystem is changed and this causes a desync in what it's supposed to do.

This PR adds a forced exit() after we redirect the user back to WP-Admin after the new branch has been put in place and options updated to make sure those actions and handlers are not executed and no errors are triggered.

To test:

Since this relies on specific conditions to be met, generic testing is a bit hard.

For the moment it would work with the following instructions:

  • Activate Bleeding edge Jetpack
  • After it's activated, Activate Feature branch fix / post by email refactoring - Post by Email: Refactoring and Unit Tests jetpack#15139
  • Without the patch this would trigger a Fatal error even in the WP-Admin UI
  • With the patch you will be redirected back to WP-Admin and the branch activated.

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This seems to work well in my tests. 🚢

@bisko bisko merged commit 7f9e3a2 into master Apr 1, 2020
@bisko bisko deleted the fix/prevent-code-execution-after-swapping-code-with-new-plugin branch April 1, 2020 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants