Skip to content

Prevent potential PHP warnings when the fs_active_plugins option comes back with a non-object#487

Closed
sc0ttkclark wants to merge 2 commits into
Freemius:masterfrom
sc0ttkclark:patch-3
Closed

Prevent potential PHP warnings when the fs_active_plugins option comes back with a non-object#487
sc0ttkclark wants to merge 2 commits into
Freemius:masterfrom
sc0ttkclark:patch-3

Conversation

@sc0ttkclark
Copy link
Copy Markdown
Contributor

This is to address an issue a Pods user encountered here: https://wordpress.org/support/topic/error-when-activating-pods/#post-14295661

@vovafeldman
Copy link
Copy Markdown
Contributor

Thanks for the PR @sc0ttkclark. Why do you suspect it is related particularly to PHP 7.4+? Did you manage to reproduce it yourself?

It looks like a storage data corruption issue.

@sc0ttkclark
Copy link
Copy Markdown
Contributor Author

I no longer have any idea why I thought it was a 7.4 issue. I'll remove that from the note on this.

As for a storage data corruption issue, I hope you're still willing to accept the PR as I believe Freemius needs to be graceful in breakages like this.

@sc0ttkclark sc0ttkclark changed the title Prevent potential PHP 7.4+ warnings when the fs_active_plugins option comes back with a non-object Prevent potential PHP warnings when the fs_active_plugins option comes back with a non-object Aug 10, 2021
Comment thread start.php Outdated
@vovafeldman
Copy link
Copy Markdown
Contributor

Thanks for checking, @sc0ttkclark. I don't think that "muting" issues, including data corruption, is the way to go. Unexpected issues are symptoms of a problem that needs to be fixed (either in the SDK or the user's site). Without being aware of it, the site owner (or us) won't know about it. If WP wasn't self-hosted we could have automatically logged the error to our end and monitor unexpected issues, but unfortunately, in the self-hosted ecosystem, the site admin is the one that needs to know about them to report them forward. Also, theoretically, data corruption can happen on every load from the DB. Adding failover logic to every DB load isn't right, there should be some level of trust between the logic and storage about data integrity.

We are happy to investigate the issue for that particular user, you can direct them to our support.

@sc0ttkclark
Copy link
Copy Markdown
Contributor Author

Look at the next line after my proposed change. Is that not muting a similar issue?

@vovafeldman
Copy link
Copy Markdown
Contributor

The purpose of the next line is to set the property when the object is created for the first time (i.e, wasn't yet stored in the DB).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants