Bug #100871
closed
Common symfony command exceptions should not be thrown
100%
Description
With #99912, setCatchExceptions(false) has been implemented to TYPO3\CMS\Core\Console\CommandApplication, so that exceptions from symfony console are handled by TYPO3 exception handlers. That change made it possible to log CLI errors in TYPO3 logs, but after thinking more about the change, I see some downsides in #99912 as pointed out below:
- CLI now throws all symfony console exceptions, which does not look pretty
- Possible user errors (e.g. unknown command or invalid amount of arguments) are now logged to the TYPO3 log
Example: ./typo3/sysext/core/bin/typo3 site:show
Uncaught TYPO3 Exception Not enough arguments (missing: "identifier"). thrown in file /path/to/typo3/vendor/symfony/console/Input/Input.php in line 70
Also note, that #100059 fixes the described problem about the not registered listener in #99912. So it is now possible to register event listeners, who actually catch all CLI errors.
I'm currently not really sure, what is best to resolve the described downsides. We could either catch common/known symfony console exceptions and print the message in a "pretty" way or we could revert #99912 and implement an event listener for ConsoleErrorEvent events, which logs those errors to the TYPO3 log.
Updated by Torben Hansen almost 1 year ago
- Related to Bug #99912: ProductionExceptionHandler does not handle CLI Exceptions added
Updated by Christoph Lehmann 12 months ago
If it's now possible to use a ConsoleErrorEventListener, then we could react on interactive mode (-n flag) and "forward" Exceptions to the ProductionExceptionHandler in non-interactive mode only. With the downside users possibly need to configure the flag for e.g. typo3/sysext/core/bin/typo3 scheduler:run (?)
A more beautiful formatting would be nice to have.
I have no opinion or rather don't care about the superfluous log messages in error log due to console errors like it is at the moment, because when something faulty happens in prod it's better to know it, even if it's due to wrong user input.
Updated by Torben Hansen 12 months ago
I created an event listener which listens on ConsoleErrorEvent events and logs exceptions using AbstractExceptionHandler::writeLogEntries(), but this also has some downsides:
- All exceptions will be logged to sys_log and TYPO3 log (this includes possible user input failures)
- The event listener is not really pretty, because in order to execute TYPO3 exception handling functionality for CLI (
@AbstractExceptionHandler::handleException()), the current exception handler must be set/restored
I therefore suggest to revert #99912. If logging of CLI exceptions is required by 3rd party code (e.g. Sentry extension), it is recommended that the 3rd party application listens on ConsoleErrorEvent to process logging of thrown console exceptions.
Updated by Gerrit Code Review 12 months ago
- Status changed from New to Under Review
Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79060
Updated by Gerrit Code Review 12 months ago
Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79060
Updated by Gerrit Code Review 12 months ago
Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79060
Updated by Gerrit Code Review 12 months ago
Patch set 4 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79060
Updated by Gerrit Code Review 12 months ago
Patch set 1 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79074
Updated by Torben Hansen 12 months ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 7fe633935a929ba5a27df12076badb857d5bd216.