search

nyurik / OsmWikibase

Wikibase customizations for OpenStreetMap
MIT License
1 stars 2 forks source link

OsmWikibase normalizePageName PHP error #1

Closed Firefishy closed 1 year ago

Firefishy commented 2 years ago

We are getting the following warning thrown on wiki.openstreetmap.org

AH01071: Got error 'PHP message: 
PHP Warning:  Declaration of OsmWikibase\\OsmSite::normalizePageName($pageName) should be compatible with Site::normalizePageName($pageName, $followRedirect = MediaWiki\\Site\\MediaWikiPageNameNormalizer::FOLLOW_REDIRECT) in /srv/wiki.openstreetmap.org/w/extensions/OsmWikibase/includes/OsmSite.php on line 43'
Firefishy commented 1 year ago 
AH01071: Got error 'PHP message: PHP Fatal error:  Declaration of OsmWikibase\\OsmSite::normalizePageName($pageName) must be compatible with Site::normalizePageName($pageName, $followRedirect = MediaWiki\\Site\\MediaWikiPageNameNormalizer::FOLLOW_REDIRECT) in /srv/wiki.openstreetmap.org/w/extensions/OsmWikibase/includes/OsmSite.php on line 43'

A fatal error is now thrown. To keep the wiki running I have disabled the extension.

tomhughes commented 1 year ago

PHP 8 issue? I didn't see that on the old machine...

Firefishy commented 1 year ago

Yes, most likely a warning become an error

Firefishy commented 1 year ago

Looks like OsmWikibase\\OsmSite::normalizePageName($pageName) needs support for the extra $followRedirect = MediaWiki\\Site\\MediaWikiPageNameNormalizer::FOLLOW_REDIRECT parameter.

manicki commented 1 year ago

To avoid bugs like this only surfacing after updating software, or the environment in runs with (e.g. php version change) it might be worthwhile to consider adding some automated tests before updating the production site? It is far from being a role model implementation but WMDE has used Github actions to run automated tests against mediawiki+Wikibase, see https://github.com/wikimedia/Wikibase/blob/901de4188b3375b37646c4b09084b3a10a6a3105/.github/workflows/secondaryCI.yml It could provide some initial inspiration on how to approach it.

fititnt commented 1 year ago

@manicki some type of errors may be viable to do make automated tests, however unless is something simple, that may take like 70% of big errors (like a bootstrap entire site and run something to emulate a browser and expect pages return HTTP 200, maybe not have string that are the defaults from PHP like Warning:...) it may get more time to create the tests than wait for user feedback.

So, I'm not saying that automated tests are not worth it, but instead of writing just because of this extension (or are very detailed), it may be better to write in general for the Wiki (aka fetch some pages that are likely to signal errors), maybe even not requiring PHP at all.

One example of a generic tool for this (there's Selenium, Puppeteer, etc) that I sometimes use is the https://pyinfra.com/. The tests can run on a local machine, but can access the remote machine (is more like Ansible, which is similar to Chef, however does not require installing an agent on remote machine, could run on average SSH connection).

I do make use of other PHP-related projects (mostly Joomla, Moodle, Wordpress, not so much MediaWiki), however things tend to work out of the box. And when they go wrong, it might be simpler to do a human review than create tests for it, because unless the tests are generic, the likelihood of the very same error happening again can take several years. So, unless something goes wrong at least 2 times, I would even write tests for it, because the tests start failing with false positives.