* Extend ConsoleLogger to support filtering by log level and
forwarding to another logger.
* Extend ConsoleSpi to support filtering by channel/level and
forwarding to another provider.
* Allow configuring which channels shell.php should log to STDERR
and at what levels.
* Deprecate shell.php -d, use more meaningful parameter names.
* Change logging behavior so that logging to STDERR doesn't
prevent normal logging, which seemed like confusing behavior
(either we should always disable normal logging for shell.php
or never, it shouldn't depend on whether we log to console).
Keep old behavior for -d, just in case.
Change-Id: I5d29c95995bf83784c96dd40df9c96e22b9ec73e
Notable changes:
* In SqlBagOStuff::getConnectionFromServerInfo, only two loggers were
injected. The rest implicitly got a NullLogger due to being absent.
These are now effectively unsilenced.
* Database::__construct() required almost all parameters, even the
loggers. I've wanted to move some of DatabaseFactory into the ctor
here for a while. In order to make this change not a breaking
change, the new 'logger' parameter is optional with NullLogger as
default. This allowed some of the test cases, which were simply
passing NullLogger, to be fixed by passing nothing instead of
passing the new option name.
The Database class is behind a dozen layers of indirection for
real use, so this will still be injected just fine (DBF, LB, LBF,
MWLBF, etc.).
* In LegacyLogger, the handling for $wgDBerrorLog was previously
limited to DBConnection and DBQuery. This now includes errors
from other (generally, newer) parts of Rdbms as well, which were
previously missing.
This only affects sites (typically CI and dev setup) where
$wgDBerrorLog is used, as opposed to the more common
$wgDebugLogGroups by-channel configuration.
* TransactionProfiler gets its logger injected in a rather odd way,
via entrypoints (MediaWiki.php, ApiMain.php, and MaintenanceRunner)
as opposed to service wiring. This is kept as-is for now.
* In LBFactoryTest, in particular testInvalidSelectDBIndependent2,
there are cases that intentionally produce failures of which
the result is then observed. In CI we assert that dberror.log is
empty so instead of adding the missing logger fields to that
LBFactory instance, the only one set (replLogger) is removed.
The alternative is to set 'logger' now, which would naturally
cause CI failures due to unexpected entries coming through to
non-mocked error log.
Bug: T320873
Change-Id: I7ca996618e41b93f488cb5c4de82000bb36e0dd3
* ForkController, OrderedStreamingForkController: indeed pcntl_fork()
can't return false.
* RL\Image: Specify type instead of using suppression, since the issue
name changes.
* VueComponentParser: Accept complaint about nullable nodeValue.
* Disable PHP 8.0 polyfill stubs when running on PHP 8.0+ to avoid
duplicate interface errors.
* Add Socket stub and use it in LegacyHandler instead of multiple
existing suppressions.
* MemcachedPeclBagOStuff: accept complaint recommending !$result over
$result === false when the type is boolean.
* MemcachedPeclBagOStuff: fix probable bug, ignoring errors from
Memcached::getMulti(). Phan noticed that $res=false was unreachable,
but it should probably be reachable.
* DatabaseMysqli: accept complaint that $this->conn->errno is already
known to be an int. It was probably a hack for some previous version
of Phan.
* BcryptPassword, MWOldPassword, MWSaltedPassword: accept complaint that
the !is_string() checks are unnecessary, after code review of PHP.
* Pbkdf2PasswordUsingHashExtension: note that contrary to Phan's
suggestion, this check is necessary.
* DefaultPreferencesFactory: remove an existing hack for
array_diff_key(), no longer necessary on 7.4 and causes an error on
8.1. Use coalesce instead of cast for the remaining
array_intersect_key() hack since it better shows that we are casting
away null.
* FullSearchResultWidget: fix likely bug involving strict comparison
between a float and an int.
* SpecialWatchlist: accept complaint that $selectedHours is
unconditionally a float, being the return value of round(), and thus
the cast is unnecessary.
* Add stub for AllowDynamicProperties, resolving an error in User.php.
* Xml: accept complaint that $encMonth is already known to be an int.
Six errors remain. These need suppressions or otherwise conflict with
PHP 7.4 support.
Bug: T322278
Change-Id: Ie375bbc8ccf22330b9a169e8da98f2bbe26ec8b9
The benefit of this setting was largely lost due to wfDebug() having
been replaced with calls to Logger->debug() for much of our code.
This is motivated by making ResourcLoader debug=2 able to make
its concurrent load.php requests more quickly during local dev.
This effectively reverts T49960. I was wrong there, thinking that
this variable has no reason for being. The vast majority of the time,
these aren't of interest on subresource requests. It's trivial
to enable when wanted, and any non-debug level messages still come
through either way, including PHP runtime errors, exceptions,
warnings, etc.
This seems to make load.php about 50% faster for local development
when using mediawiki-docker.
I used `ab` to perform a load.php request 300 times. The mean was
320-360ms before this patch (with and without DebugRawPage=false),
and around 120-140ms with patch (with DebugRawPage=false, default).
Bug: T85805
Bug: T225730
Change-Id: I061419adebf43e83cd0103bdb9b7a3296a2a1122
MediaWiki still supports PHP 7.2+, but we want to mainly test in newer versions
of PHP. Setting minimum_target_php_version to 7.2 this lets us run phan without
phan trying to get us to make PHP 7.2-incompatible changes to 'appease' PHP 8.0
or whatever later changes.
Some switches of generic 'resource' type-hinting to 'resource|object' to inform
phan to ignore this (triggering PHPCS at the time, ah well), rather than trying
to hint the specific novel PHP encapsulation classes to that have replaced them
from PHP 8.0 onwards but don't yet exist, and fixes from where we were checking
the results of implode and explode.
Bug: T293924
Change-Id: I629e3fb3adfad73beb3d424a07e643c2e079d9bb
Also remove the unmaintained kafka-php package from the from "suggested"
and "dev" composer dependencies, as this is now no longer used.
Change-Id: If5668974f417b627df95bce47db18d46fa03327c
These require use of nmred/kafka-php 0.1.0, which is from 2015.
Upstream has had one major release since then, in 2017. There have
been various commits, issues, and pull requests since but nothing
released for five years.
EventRelayer was created for use with WANObjectCache, where WMF
considered use of Kafka as one of the possible transports. This never
happened, and WANObjectCache has not supported EventRelayer since 2019,
removed with change I328eb74f21c23.
The EventRelayer abstraction itself remained unused, until it became
re-purposed for CdnCacheUpdate, which WMF now relays via EventBus.
Monolog KafkaHandler is no longer used, and was similarly created for
WMF's use case. It seems unlikely to be of interest to most third
parties. And those larger installs like WMF would likely be more
interested in SyslogHandler which is more standardised across the
industry (especially for cloud native tooling around containers).
Someone using this can continue to do this by adding a similar class
to their own code instead, possibly published as an extension.
Deprecating this in MW 1.38 (with backport) means we can remove it
shortly after in master, and no longer maintain this unused/untested
integration for which no known users exist, as well as remove our
supported suggested dependency on the unmaintained kafka-php lib.
Change-Id: I475227293ca050acead357efebce12e93a971f65
Make phan stricter about null types by setting null_casts_as_any_type to
false (the default in mediawiki-phan-config)
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T242536
Bug: T301991
Change-Id: I0f295382b96fb3be8037a01c10487d9d591e7e01
The functions returning null or the class property is set explict null
Found by phan strict checks
Change-Id: I4a271093fb6526564d8083a08249c64cb21f2453
This replaces references to DefaultSettings with
references to config-schema.yaml where appropriate.
NOTE: this does not yet change Setup.php. DefaultSettings.php
remains intact and is still being used.
NOTE: this does not remove usages in the installer, see I5d8843a1062fbf
for that.
Bug: T300129
Change-Id: Ie6152cf510c3be61bc22167ca6d90dfc28910a45
* Enforce the ban on is_resource in phpcs
* In OrderedStreamingForkController, the comment was incorrect. I
confirmed using a small test script that if the child closes one end
of a socket pair, the other end will still be open, and is_resource()
will still return true, and fclose() will not fail. The issue was
introduced in c82d30d19c, it was not present in the
CirrusSearch copy of the class.
* Allow is_resource() for debug logging.
* Allow is_resource() for parameter validation where a stream may be
passed to a function, since there is no alternative.
Bug: T260735
Change-Id: I59a752f7bb4b043ddfb2434b52a02f9221bf54db
Make getServer() always return a string, as documented, even with new
Database::NEW_UNCONNECTED handles that have yet to call open(). If the
'host' parameter to __construct() is ''/null, getServer() now returns
'localhost' instead of null. This avoids problems like fatal errors in
calls to TransactionProfiler::recordConnection().
Use Database constants for "connectionParams" field keys for better
static analysis.
Also:
* Add Database::getServerName() method that returns "readable" server
names in the style of LoadBalancer::getServerName(). Note that the
"hostName" field is already passed in from LoadBalancer.
* Migrate most getServer() callers to getServerName() for easier
debugging and more readable logging.
* Also, normalize Database/LoadBalancer SPI logging context to use
"db_server" and reduce logging code duplication in LoadBalancer.
Bug: T277056
Change-Id: I00ed4049ebb45edab1ea07561c47e226a423ea3b
This reverts the patch I124391cc7b44 because it is no more needed with
Monolog 2.x to push a NullLogger to log nothing. This patch is a
micro-optimisation for Monolog channels where nothing is logged.
Bug: T196906
Change-Id: I1f414090a37cd4d84d37a2fca8833e3623c2983f
Before this commit empty Monolog loggers (or at least when no handlers are
defined) were assigned (by Monolog 1.x) the stream handler php://stderr with
the level 'debug'. This whole behaviour from MediaWiki config to Monolog is
unexpected; in the outcomes, depending on the PHP execution program, stderr
is or is not saved somewhere by default, leading to very different behaviours.
The behaviour will change in Monolog 2.x and nothing will happen is such cases
(similar hence to a NullHandler). With this commit the behaviour is uniform
accross all environments and (future) Monolog version: nothing is logged if
no handler is defined.
Precisely, a NullHandler for a given logger/MediaWiki logging channel is
defined here either when there is no key 'handlers', either when this key is
an empty array or 'false'.
Bug: T196906
Change-Id: I124391cc7b4485081980c5015431404234f40073
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: I17e5e92e24c708ffc846945a136347670a3a20c7
Per the Stable Interface Policy, PHP interfaces should not be
directly implemented by extensions, unless they are marked to be safe
for that purpose.
Bug: T247862
Change-Id: Idd5783b70fc00c03d57f5b1a887f0e47c4d7b146
Log lines for SQL queries were not shown in either the debug toolbar's
debug log, or in the file configured by $wgDebugLogFile. I think it is
useful to have SQL queries both in a timeline context in the debug log,
and separately in table format.
This is a very similar complaint to I8edb41aa1d8236047b80830, which
affected deprecation messages.
Change-Id: Ie0ca7bf8394d3008e3f9e950c201da1a96276fea
== Motivation
Mute a log channel, for which the Logger object is injected by
service wiring, for a service that is overridden by default,
such as 'DBLoadBalancerFactory'. For that, calling setLogger()
mid-test would be too late.
== Changes
* Add a test-only method to LegacyLogger that makes it possible
to change its `minimumLevel` attribute, thus making it turn
itself into a NullLogger if raised to infinity. This is the
same principle we use already for disabled log channels when
using MediaWiki normally (see LegacyLogger::__construct).
* Previously, the developer's LocalSettings.php was loaded
which includes the Spi configuration. This meant other Spi's
could be configured which means we might not be dealing with
a LegacyLogger object.
Similar to what we do with ObjectCache and JobQueue already,
make the default Spi in tests the same as the normal MW default.
* Add setNullLogger() which makes use of these two.
Bug: T248195
Change-Id: Ieade3585812de47342259afa765e230fff06f526
PHP 7.0 makes many error conditions throw instances of the new Error class
which does not extend the known Exception.
The Throwable interface provides a concise and type-safe way of handling
either, e.g. for logging purposes, but HHVM did not support it, requiring
tedious fallback checks.
This commit replaces occurrences of Exception in code paths equally
covered by Throwable, like Exception|Throwable parameter and return types
(also nullable), instanceof guards, duplicated `catch` blocks, as well as
related comments and documentation blocks, with the exception of $previous
parameter descriptions consistent with the manual at
https://www.php.net/manual/en/exception.construct.php
Proper type declarations have been added or reinstated where possible.
Change-Id: I5d3920d3cc66936a350314e2f19c4f6faeffd7c0
Scalar casts are still allowed (for now), because there's a huge amount
of false positives. Ditto for invalid array offsets.
Thoughts about the rest: luckily, many false positives with array offsets
have gone. Moreover, since *Internal issues are suppressed in the base
config, we can remove inline suppressions.
Unfortunately, there are a couple of new issues about array additions
with only false positives, because apparently they don't take
branches into account.
Change-Id: I5a3913c6e762f77bfdae55051a395fae95d1f841
Repeating the variable name doesn't do anything. Documentation
generators don't need it. It's more stuff to read that doesn't add new
information. And it can become outdated.
Note there are two types of @var docs. When used inline (and not on a
class property) the variable name is needed.
Change-Id: If5a520405efacd8cefd90b878c999b842b91ac61
