• Resolved KZeni

    (@kzeni)


    I found that one of my sites auto-updated Feed Them Social in the past few days which resulted in a site-wide fatal server error.

    Thankfully, this is a quick fix that should be easy to accommodate to avoid this coming up in future versions.

    The fatal server error that was happening site-wide was:

    Parse error: syntax error, unexpected ')' in /wp-content/plugins/feed-them-social/admin/cpt/access-tokens/single/twitter-access-token.php on line 221

    In short, editing /wp-content/plugins/feed-them-social/admin/cpt/access-tokens/single/twitter-access-token.php so that lines 215-221 of:

    echo sprintf(
        esc_html__( '%1$s%2$s Please click the Login and Get my Access Token button again.%3$s', 'feed-them-social' ),
        '<div class="fts-failed-api-token">',
        $data->error->message,
            '</div>',
    
    );

    is instead:

    echo sprintf(
        esc_html__( '%1$s%2$s Please click the Login and Get my Access Token button again.%3$s', 'feed-them-social' ),
        '<div class="fts-failed-api-token">',
        $data->error->message,
            '</div>'
    
    );

    resolves the issue.

    The only thing that changed was removing the unnecessary trailing comma in the list of parameters when calling the sprintf function (it’s already being called elsewhere without the trailing comma so this instance was already going against what the norm was for this plugin with this seeming to be the only place this is occurring [at least the site-wide server error went away after making this one change.])

    While that trailing comma can be harmless in some PHP versions, it does cause fatal server issues in PHP 7.2 & potentially others. That makes this a straightforward fix to implement for future plugin releases considering that comma isn’t doing anything anyway when it just adds the possibility of causing errors (while trailing commas in arrays, etc. is generally okay, it seems calling functions was sensitive to trailing commas in these particular PHP versions.)

    It seems future updates just need to be a bit more mindful of this and/or be tested against that potential compatibility issue (known to be problematic for PHP 7.2) before release as a site-wide server error certainly is serious.

    Meanwhile, if guaranteeing that ongoing PHP version support without a future release potentially having something like this slip through isn’t realistically manageable, I can definitely see this plugin updating its required PHP version to something like PHP 7.3+ (or possibly even newer) instead of the PHP 7.0+ requirement it has now (where it effectively said the plugin supports PHP 7.2 while it then had a PHP 7.2 compatibility issue introduced in one of the recent updates [seems it wasn’t added in the latest update since I fixed the issue and then updated the plugin again to find the issue was re-introduced per that version still having this issue.]) This certainly wouldn’t be the first plugin to push the PHP version requirement to a more modern expectation. That being said, I would highly recommend releasing a version with this fix before updating the PHP version requirement so sites with these older PHP versions can get one last good version release of this plugin before they get locked out of future updates of the plugin.

Viewing 3 replies - 1 through 3 (of 3 total)
  • @kzeni — excellent work and post. I was very relieved to find this here today, and I’m glad you shared a solution.

    Plugin Author slickremix

    (@slickremix)

    @kzeni ah, thank you for reporting this. I will get a push here in a bit to correct this. I looked, and it shows a warning in my phpStorm about it affecting 7.2, so I will watch for these in the future. There is no reason to have a trailing comma anyway. Thanks again.

    Spencer

    Plugin Author slickremix

    (@slickremix)

    I have pushed an update to correct this error. Thanks again for reporting it.

    Spencer

Viewing 3 replies - 1 through 3 (of 3 total)
  • You must be logged in to reply to this topic.