Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: PHP error - Call to a member function addTranslationFile() on null #18699

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Rom1-B
Copy link
Contributor

@Rom1-B Rom1-B commented Jan 10, 2025

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

Prevent error message :

glpiphplog.CRITICAL: Uncaught Error: Call to a member function addTranslationFile() on null {"exception":"[object] (Error(code: 0): Call to a member function addTranslationFile() on null at .../src/Plugin.php:480)"} 
glpiphplog.CRITICAL:   *** Uncaught Exception Error: Call to a member function addTranslationFile() on null in .../src/Plugin.php at line 480
  Backtrace :
  src/Plugin.php:1710                                Plugin::loadLang()
  src/Plugin.php:557                                 Plugin->getInformationsFromDirectory()
  src/Plugin.php:606                                 Plugin->getPluginInformation()
  src/Plugin.php:545                                 Plugin->checkPluginState()
  src/Plugin.php:294                                 Plugin->checkStates()
  src/Glpi/Kernel/Listener/InitializePlugins.php:69  Plugin->init()
  .../event-dispatcher/Debug/WrappedListener.php:116 Glpi\Kernel\Listener\InitializePlugins->onPostBoot()
  ...ymfony/event-dispatcher/EventDispatcher.php:220 Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke()
  ...symfony/event-dispatcher/EventDispatcher.php:56 Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
  ...spatcher/Debug/TraceableEventDispatcher.php:139 Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
  src/Glpi/Kernel/Kernel.php:118                     Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch()
  src/Glpi/Console/Application.php:108               Glpi\Kernel\Kernel->boot()
  bin/console:156                                    Glpi\Console\Application->__construct()

Screenshots (if appropriate):

@@ -4681,6 +4681,12 @@
'count' => 2,
'path' => __DIR__ . '/src/Plugin.php',
];
$ignoreErrors[] = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

baseline is only present so we do not have to fax all existing issues; we should not add new ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is a copy/paste from what's in src/Session.php, which is also in the baseline, that's why I tried to ignore it in the same way, to see if it's accepted, but apparently not 😢

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware we already have an occurrence of that... But, the fact a code already exists does not mean it's correct :D
Keep in mind we'll have to fix phpstan issues one day.

@trasher
Copy link
Contributor

trasher commented Jan 10, 2025

Seems like this affects existing test (no idea if tests were wrong before - I did not check).

I'm not sure to understand the fix anyway; but I do no know this part really well.

@AdrienClairembault
Copy link
Contributor

It seems plugins are loaded before the session language is set, I guess the LoadLanguage kernel listener should have higher priority than the InitializePlugins listener.

@AdrienClairembault
Copy link
Contributor

Can you try something like this ?

diff --git a/src/Glpi/Kernel/ListenersPriority.php b/src/Glpi/Kernel/ListenersPriority.php
index 08b036052c..e33b4f793c 100644
--- a/src/Glpi/Kernel/ListenersPriority.php
+++ b/src/Glpi/Kernel/ListenersPriority.php
@@ -46,9 +46,9 @@ final class ListenersPriority
         KernelListener\InitializeCache::class =>                     170,
         KernelListener\LoadLegacyConfiguration::class =>             160,
         KernelListener\CustomObjectsAutoloaderRegistration::class => 150,
+        KernelListener\LoadLanguage::class =>                        145,
         KernelListener\InitializePlugins::class =>                   140,
         KernelListener\CustomObjectsBootstrap::class =>              130,
-        KernelListener\LoadLanguage::class =>                        120,
     ];

     public const REQUEST_LISTENERS_PRIORITIES = [

@AdrienClairembault
Copy link
Contributor

And if it works, we can probably add some kind of test to make sure it stays working.

src/Plugin.php Outdated
Comment on lines 480 to 490
if (!defined('TU_USER')) {
$i18n_cache = new I18nCache((new CacheManager())->getTranslationsCacheInstance());
$TRANSLATE = new class ($i18n_cache) extends Laminas\I18n\Translator\Translator {
public function __construct(?I18nCache $cache)
{
$this->cache = $cache;
}
};
$TRANSLATE->setLocale($coretrytoload);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the listener change is effective, this can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this code, the initial error comes back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works on my side, but only after deleting the cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes for me too by emptying the folder _cache

@AdrienClairembault
Copy link
Contributor

Regarding the test, the only way to reproduce the issue is to have active plugin with at least one locale file.

The simplest way do to it is I guess to add some locales files to the fake tester plugin, this way all tests will fail as the kernel won't be able to boot properly:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants