Page MenuHomePhabricator

PHP Notice: Undefined variable: values at PageForms/includes/PF_FormField.php:354
Closed, ResolvedPublic


PageForm is installed on and after updating to the master version, we started encountering the following error,

[2019-07-15 10:26:35] error.ERROR: [b2685845050fdc1175d68716] [no req]   ErrorException from line 354 of /srv/mediawiki/tags/2019-07-15_10:19:45/extensions/PageForms/includes/PF_FormField.php: PHP Notice: Undefined variable: values {"exception":"[object] (ErrorException(code: 0): PHP Notice: Undefined variable: values at /srv/mediawiki/tags/2019-07-15_10:19:45/extensions/PageForms/includes/PF_FormField.php:354)","exception_id":"b2685845050fdc1175d68716","exception_url":"[no req]","caught_by":"mwe_handler"} []
[15-Jul-2019 10:26:35 UTC] PHP Notice:  Undefined variable: values in /srv/mediawiki/tags/2019-07-15_10:19:45/extensions/PageForms/includes/PF_FormField.php on line 354

PageForm version: master
MediaWiki version: 1.34.0-alpha

Event Timeline

Eeeeeek, this error suggested me to add phan to PageForms, in order to catch this kind of stupid mistakes early on during development. Then I saw, and it frightened me. This extension tries to use methods removed from core in 1.32, and that's just the first line of a huge report. Plus, T149869 scares me a little bit.
I'm unsure about what to do here. First of all, the extension claims to support MW 1.23+ at, although extension.json says 1.27+. Either way, those are way too old to kept compatibility with them without tons of back-compat code etc.
I'll probably open another task for that, although given all of the above, I think the undefined variable could be the least of the problems this extension has.

I think the undefined variable could be the least of the problems this extension has.

Operations-wise, log spam is the main issue we are having with this extension currently. But it's true that this extension has fallen behind the contemporary MediaWiki extension standards, and feels like coming apart in other ways as well. @Yaron_Koren are you aware of this and do you have plans how to rectify this? A huge lot of sites depend on this extension.

Operations-wise, log spam is the main issue we are having with this extension currently.

Yeah, but log spam coming from calls to undeclared methods (removed several MW versions ago) is IMHO more critical, isn't it? :-)

Note, I filed T228057 as wider-scope task, and in the meanwhile I'll start fixing some obvious errors.

@Nikerabbit - what do you mean by that?

I think this specific issue was fixed about a month ago, by the way, in 94371811cb0d - unless you're already using a more recent version of Page Forms.

Nikerabbit claimed this task.

Yup. I was firefighting yesterday with multiple errors after update of code. I did update PageForms to latest version in the hurry to solve this issue, but I though it didn't work, because I got errors after that too. Turns out it was just due to delay in my error log relay and the notices stopped after that.

@Nikerabbit - what do you mean by that?

I observe:

  • Frequent breakage with new merged commits (like this issue)
  • Frequent issues from core changes (depreciation notices, or even errors after deprecation has gone through). This is to some extent caused by the unclear BC policy which makes other developers to avoid touching your code.
  • The code base still using PHP <5.4 long array syntax and claims to support PHP5. Last MW version to support PHP 5.4 went out of support in November 2016!
  • The code base is using a lot of weak comparisons (==, !=) instead of strict comparisons (===, !==)
  • Lots of very long and complex functions without tests, that makes it difficult to change anything (I've tried to fix a few bugs, but it's really hard)

Here are some suggestions:

  • Set a clear and reasonable backwards compatibility policy
  • Reduce the number of disabled sniffs in codesniffer (and update codesniffer)
  • Replace jshint with eslint like has been done in other extensions
  • Add phan, maybe even sonarcube in the future
  • Adopt
  • Start increasing test coverage for new and changed code where it can be done with reasonable effort

@Nikerabbit - thanks for that explanation. Some of these tasks, there are already plans to implement, like adding in more tests. I'd say more, but I don't know if a closed bug report is the best place to have this kind of discussion. :)