E_STRICT is deprecated in PHP 8.4.0.
MediaWiki (and almost all extensions) no longer produces errors at this
level.
To not break the compatibility, let's converts all E_STRICT level errors
to E_USER_NOTICE, which is also mapped to warning severity.
Using '@' operator to avoid generating the deprecation warning of
E_STRICT.
This should addresses the deprecation warning of E_STRICT when another
warning or error is raised.
Bug: T375707
Change-Id: I5937cacdf5b01614042a06d4deb5112ffff51727
If an exception is thrown or an error is raised during HookContainer
construction, don't be surprised when you can't get a HookContainer.
When checking if we need to roll back transactions, use peekService()
instead of isDisabled(). According to the disableService()
documentation, peekService() will always return null for a disabled
service. If the service hasn't been created, there can be no
transactions open and so there is no point in rolling back.
It's not necessary to catch DBError exceptions from peekService(),
since it never calls the instantiator.
Fix phan error by broadening the onLogException() parameter type. Phan
is correct, the parameter is not guaranteed to be
Exception|ErrorException and so the previous documentation was wrong.
Bug: T379125
Change-Id: Ie5ec2bd1ecfb0114f3d00fc28b2c5e3db9bd87c0
(cherry picked from commit 5cf3cd03dfd3f7acca4d78bf12054f3c315fee28)
Also remove @var from const, the type is given by the value of the const
and does not need documentation.
Change-Id: I65636f2b97b44387c4320ebfb4be6c81d6f771b9
get_debug_type() does the same thing but better (spelling type names
in the same way as in type declarations, and including names of
object classes and resource types). It was added in PHP 8, but the
symfony/polyfill-php80 package provides it while we still support 7.4.
Also remove uses of get_class() and get_resource_type() where the new
method already provides the same information.
For reference:
https://www.php.net/manual/en/function.get-debug-type.phphttps://www.php.net/manual/en/function.gettype.php
In this commit I'm only changing code where it looks like the result
is used only for some king of debug, log, or test output. This
probably won't break anything important, but I'm not sure whether
anything might depend on the exact values.
Change-Id: I7c1f0a8f669228643e86f8e511c0e26a2edb2948
This patch introduces a namespace declaration for the
MediaWiki\Json to FormatJson and establishes a class
alias marked as deprecated since version 1.43.
Bug: T353458
Change-Id: I5e1311e4eb7a878a7db319b725ae262f40671c32
This was introduced to allow maintainers during local development,
and e.g. when debugging in production, to capture silenced runtime
warnings, such as those silenced via AtEase or the `@` PHP operator.
These are not expected to be of interest to capture by default on
web requests, hence placed in a separate channel that e.g. one would
only enable during development, or when debugging.
When this was introduced, MediaWiki was only just starting to
experiment with PSR-3 structuerd logging, and thus it hardcoded JSON.
Now that we support structured logging natively, this has been
obsoleted by the "error" channel (for warnings in production),
and (since last December) the "silenced-error" channel (for debugging
and development) with commit f6cb39979b (I34b19837c3).
With the above two alternatives available, there is no longer a need
for the duplicate "error-json" channel.
Bug: T193472
Change-Id: I7a6e6fa52a47a29ec04411a6c8b05e005a0a4fc7
It's now deprecated to use the individual error printing functions in
MWException or to override them. Printing MWException now works the
same as any other exception, and happens in MWExceptionRenderer.
It's still allowed to override MWException::report() to replace the
entire error page, for example in ErrorPageError.
Bug: T353444
Change-Id: I66d896f6f229b90e6ba9949311b56a6b6ab3da3d
Replace some other global variable checks with MW_ENTRY_POINT too.
This unfortunately makes it difficult to override the result of
isCommandLine() for testing. However, the existing tests weren't
really checking anything useful, but rather just duplicated the real
code, so I won't miss them.
Bug: T313841
Change-Id: I8bc200d67877ce04f80fd587481a76c7df629d51
Follows-up 4f08bb3be9 (I240b75f5a4), which improved the "error" channel
to make use of the PSR-3 severity level (instead of defaulting to
ERROR).
This change exposes the suppressed errors to a new channel,
with a forced severity level DEBUG.
The formatted message is unchanged and reflects the original severity
through its "message" (i.e. "PHP Notice", "PHP Warning", etc.)
Bug: T193472
Change-Id: I34b19837c391ff4acdb55fc0c310c0f554f15da2
The base phan config uses a file_exists check to determine whether to
use the namespaced class name, but it doesn't work when running against
core because MW_INSTALL_PATH isn't set. So specify the type in the local
config, and remove @phan-var annotations added in I6bbdbbe6.
Also use `::class` instead of string literals for classes.
Change-Id: I994a0ed32ea948253ed07ee3cc8868a0eaa6d8b9
The Hooks class contains deprecated functions and the whole class is
going to get removed, so remove the convenience function and inline the
code.
Bug: T335536
Change-Id: I8ef3468a64a0199996f26ef293543fcacdf2797f
If the exception handler gets triggered before a service container is
available, it should not attempt a DB rollback. Attempting to access
the DBLoadBalancerFactory service object would cause a secondary error,
obscuring the original issue.
This might happen when the exception handler is triggered during
bootstrapping or in the installer.
Change-Id: I644bd0953aa5e690fea16d9fc11ca3f24cb3f104
* Replace is_/throw with native where possible.
* Prefer strict comparisons where possible.
* Remove `@throws` for exceptions that are not meant to be checked
or caught by callers.
* Make the separateCacheableRows() return hint more precise.
Change-Id: I1c14bb8faaf1b377b6d179d96e18331acff23c5b
This reverts parts of I8520d8cb16 and Ib941c22d6b7e.
The documentation of ShowExceptionDetails, as well as all other
uses of it (e.g. MWExeceptionRenderer for index.php, API, and
ResourceLoader) take it to mean to hide both exception message and
details.
This is why MWExceptionHandler didn't have, and didn't need,
the added complexity of this as a parameter as this method
simply wouldn't be called at all in that case.
* Rename the method added in I8520d8cb16 to match the one
in MWExceptionRenderer.
* Update REST handling to now print any exception details
when it is true.
* Remove the now-unused code introduced in Ib941c22d6b7e.
Change-Id: I1a9920dea0bafe315a20489efbe46ea9b55b0f74
Allow callers of MWExceptionHandler::getStructuredExceptionData() and
jsonSerializeException() to explicitly control whether a backtrace is
included in the return value. This avoids the need to rely on the
LogExceptionBacktrace setting in static methods.
Bug: T294739
Change-Id: Ib941c22d6b7ec5f1b984bf5ded90652e42ad7b67
Rename canRecoverFromDisconnect() in order to better describe
its function. Make it use the transaction ID and query walltime
as arguments and return an ERR_* class constant instead of a bool.
Avoid retries of slow queries that yield lost connection errors.
Track session state errors caused by the loss of named locks or
temp tables (e.g. during connection loss). Such errors will prevent
further queries except for rollback() and flushSession(), which must
be issued to resolve the error.
Add flushPrimarySessions() methods to LBFactory/LoadBalancer
and use it in places where session state loss is meant to be
safely aknowledged.
Change-Id: I60532f86e629c83b357d4832d1963eca17752944
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
Rename canRecoverFromDisconnect() in order to better describe
its function. Make it use the transaction ID and query walltime
as arguments and return an ERR_* class constant instead of a bool.
Avoid retries of slow queries that yield lost connection errors.
Add methods and class constants to track session state errors
caused by the loss of named locks or temp tables. Such errors can
be resolved by a "session flush" method.
Make assertQueryIsCurrentlyAllowed() better distinguish ROLLBACK
queries from ROLLBACK TO SAVEPOINT queries. For some scenarios,
only full tranasction ROLLBACK queries should be allowed.
Add flushSession() method to Database and flushPrimarySessions()
methods to LBFactory/LoadBalancer.
Also:
* Rename wasKnownStatementRollbackError() and make it take the
error number as an argument, similar to wasConnectionError().
Add mysql error codes for query timeouts since they only cause
statement rollbacks.
* Rename wasConnectionError() and mark it as protected. This is an
internal method with no outside callers.
* Rename wasQueryTimeout(), remove some HHVM-specific code, and
simplify the arguments.
* Make executeQuery() use a for loop for the query retry logic
to reduce code duplication.
* Move the error state setting logic in executeQueryAttempt() up
in order to reduce code duplication.
* Move the beginIfImplied() call in executeQueryAttempt() up to the
retry loop in executeQuery(). This narrows the executeQueryAttempt()
concerns to sending a single query and updating tracking fields.
* Make closeConnection() and doHandleSessionLossPreconnect() in
DatabaseSqlite more consistent with the base class by releasing named locks.
* Mark trxStatus() as @internal.
Bug: T281451
Bug: T293859
Change-Id: I200f90e413b8a725828745f81925b54985c72180
The functions returning null or the class property is set explict null
Found by phan strict checks
Change-Id: I4a271093fb6526564d8083a08249c64cb21f2453
This helps phan to detect unreachable code and also impossible types
after the functions.
It helps phan to avoid false positives for array keys
when the keys are checked before
Bug: T240141
Change-Id: I895f70e82b3053a46cd44135b15437e6f82a07b2
Log INormalizedException messages in a structured way, allowing
the logging infrastructure to group them better.
Change-Id: I877909d1113ab93b4b8a115af5bb0fe039ea32d6
Follows-up f2543d442a, in which I accidentally removed the prefix.
Also, while at it, drop the "$class: " prefix that we normally
add to exceptions. For errors we internally construct ErrorException,
but this is something a clas name rarely seen by PHP developers and
has sometimes led to confusions about whether this is an exception
or not. We did still have "PHP Notice" and "PHP Warning" right after
it (except for deprecations) which should act as confidence indicator
that they are in fact not exceptions. We also have the channel
name ("error" instead of "exception"). But removing this will help
boost that confidence further, plus it had no added value anyhow, so
less is more?
Change-Id: Ib93f48c94d642f519558aab40d143b43e1d9ed16
This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.
Created by I25a17fb22b6b669e817317a0f45051ae9c608208
Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
This key predates MediaWiki's standardised notion of request IDs,
and of PSR-3 and Monolog usage.
Bug: T199607
Change-Id: Ibdd5bf12591761ab45be12ba72943e9d94f678cb
This should make error logs easier to work with through a couple
of ways:
* The stack trace is now complete, instead of missing the first
crucial step, which is often the one used for filtering
purposes and for identifying errors within a given deployed
version of MediaWiki. (E.g. when filtering out an error that is
expected to be fixed by the next release and/or when checking
how prominent an error currently is).
* Logstash reports that report message + trace will not need to be
edited by hand to include the file+line.
* The workflow for Logstash generally follows one of two patterns.
The default is to filter by exception.file (including line number),
which is very sure to catch all possible variants thrown from
the same code, regardless of any variables in the message, but
has the downside of not matching week-over-week consistency due to
file paths (at least for WMF) containing the deployment version.
The other option is to filter by message, which has the risk of
possibly excluding too much if there are multiple unrelated ways
to trigger the issue, but is a sensible second option. This is
usually done by filtering on normalized_message for non-exception
errors, but doesn't work well for exceptions because they contain
the file paths and do so in-between the class and message words,
and thus are not compatible with Logstash's default substring/term
match. The alternative of exception.message is then considered but
is lacking the class/type, which can be fragile.
With this change applied, no editing is needed, and no multiple
approaches need to be considered with the same option.
Either filtering by exception.file as-is, or filtering by
normalized_message as-is, regardless of whether it is an exception
error or other message in another channel, will both work.
Bug: T271496
Change-Id: I5908ed53f9b97b3c9cde126aca89ab6fc197c845
A PHP fatal error from MWExceptionRenderer::output() causes the
exception handler to be re-entered, with the original exception thrown
away. So, catch all throwables. That way we can see both error messages,
neither is thrown away.
Bug: T263911
Change-Id: Ie98438cbdd328fe295c9b5202d79edb0c8fb41c5
