IBX-11507: Critical errors logged when browsing BackOffice as user with limited permissions#100
IBX-11507: Critical errors logged when browsing BackOffice as user with limited permissions#100Sztig wants to merge 3 commits into
Conversation
|
This is no way to go, we cant just disable all logging. Is it possible to use or even better maybe we can easly configure it to log those into specific channel? |
|
I've done some test and something like this seem to work - how about we log it under the default |
|
@Sztig if what you meant is logging GraphQL issues in a separate channel then I think we should indeed go for it. |
|
Also, CI on PHP 7.4 is fixed, feel free to rebase this PR. |
Sztig
force-pushed
the
IBX-11507-log-pollution-fix
branch
from
271ebdb to
9b5d841
Compare
|
So I have went with this approach - I have added new channel called |
| 'dir' => $container->getParameter('kernel.project_dir') . self::SCHEMA_DIR_PATH, | ||
| ]; | ||
| $container->prependExtensionConfig('overblog_graphql', $graphQLConfig); | ||
| $container->prependExtensionConfig('monolog', ['channels' => ['graphql']]); |
There was a problem hiding this comment.
This is not necessary, if you use autowiring/autoconfiguration from MonologBundle, AFAIR, see the next diff comment.
|
|
||
| Ibexa\GraphQL\EventListener\ArgumentsExceptionListener: | ||
| arguments: | ||
| $logger: '@monolog.logger.graphql' |
There was a problem hiding this comment.
Try rather tagging this as
tags:
- { name: monolog.logger, channel: ibexa.graphql }Notice that I changed the channel name to ibexa.graphql so it follows the Product convention.
What I don't remember is if you need to use LoggerAwareTrait and implement LoggerAwareInterface for these pieces to work together. If it can be avoided and constructor can inject non-nullable LoggerInterface, then let's skip it.
| return; | ||
| } | ||
|
|
||
| $this->logger->critical( |
There was a problem hiding this comment.
I see there was a discussion about whether it should be critical or debug. To me it feels like a debug if it comes from lack of permissions.
|
5 participants
Description:
This is a "hotfix" for excessive error logging when using backoffice as a user with limited permissions. The fix is not perfect as it silences the logging entirely.
Alternatives considered:
Make ArgumentsException extend UserWarning and remove the previous exception
Catch ArgumentsException in resolvers and re-throw as UserError/return null
The underlying issue is that we are no matter what querying all the info about content so depending on the permissions, other users will throw different errors in the logs while the backoffice functions as intended.
Fixing this by checking permissions seems to be too expensive/complex of a fix.