RotherOSS / otobo

OTOBO is one of the most flexible web-based ticketing systems used for Customer Service, Help Desk, IT Service Management. https://otobo.io/
GNU General Public License v3.0
248 stars 71 forks source link

Modul refresh fails after a module could not be loaded once #1944

Open bschmalhofer opened 2 years ago

bschmalhofer commented 2 years ago

This is only relevant during development. I noticed that modules that were changed on disk, 10 s ago, are not releaded. @svenoe suggested that reloading works in most cases, but not when the module had compile errors once.

bschmalhofer commented 5 months ago

Encountered this effect again after copying a broken Perl module into the Docker container. There was no automatic recovery after the code was fixed. After docker compose restart web everything was fine again. Here is the log:

ERROR: OTOBO-CGI-10 Perl: 5.38.2 OS: linux Time: Thu Apr  4 13:32:10 2024

 Message: Attempt to reload Kernel/System/DynamicField/Driver/ConfigItem.pm aborted.
Compilation failed in require at /opt/otobo/bin/psgi-bin/../../Kernel/System/Main.pm line 112.

 RemoteAddress: 172.23.0.1
 RequestURI: /otobo/index.pl?Action=AdminDynamicField;ObjectTypeFilter=ITSMConfigItem;NamespaceFilter=Server

 Traceback (26): 
   Module: Kernel::System::DynamicField::Backend::new Line: 104
   Module: Kernel::System::ObjectManager::_ObjectBuild Line: 344
   Module: Kernel::System::ObjectManager::Get Line: 216
   Module: Kernel::System::Ticket::TicketSearch::TicketSearch Line: 1454
   Module: Kernel::Output::HTML::ToolBar::TicketLocked::Run Line: 58
   Module: Kernel::Output::HTML::Layout::Header Line: 1492
   Module: Kernel::Modules::AdminDynamicField::_ShowOverview Line: 176
   Module: Kernel::Modules::AdminDynamicField::Run Line: 84
   Module: Kernel::System::Web::InterfaceAgent::Content Line: 1247
   Module: Kernel::System::Web::InterfaceAgent::Response Line: 1329
   Module: Kernel::System::Web::App::call Line: 86
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::App::URLMap::call Line: 71
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 212
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 182
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 168
   Module: Plack::Middleware::HTTPExceptions::try {...}  Line: 20

ERROR: OTOBO-CGI-10 Perl: 5.38.2 OS: linux Time: Thu Apr  4 13:32:10 2024

 Message: Can't load dynamic field backend module for field type ConfigItem!

 RemoteAddress: 172.23.0.1
 RequestURI: /otobo/index.pl?Action=AdminDynamicField;ObjectTypeFilter=ITSMConfigItem;NamespaceFilter=Server

 Traceback (26): 
   Module: Kernel::System::DynamicField::Backend::new Line: 105
   Module: Kernel::System::ObjectManager::_ObjectBuild Line: 344
   Module: Kernel::System::ObjectManager::Get Line: 216
   Module: Kernel::System::Ticket::TicketSearch::TicketSearch Line: 1454
   Module: Kernel::Output::HTML::ToolBar::TicketLocked::Run Line: 58
   Module: Kernel::Output::HTML::Layout::Header Line: 1492
   Module: Kernel::Modules::AdminDynamicField::_ShowOverview Line: 176
   Module: Kernel::Modules::AdminDynamicField::Run Line: 84
   Module: Kernel::System::Web::InterfaceAgent::Content Line: 1247
   Module: Kernel::System::Web::InterfaceAgent::Response Line: 1329
   Module: Kernel::System::Web::App::call Line: 86
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::App::URLMap::call Line: 71
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 212
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 182
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 168
   Module: Plack::Middleware::HTTPExceptions::try {...}  Line: 20
bschmalhofer commented 3 months ago

I still do not have a good idea how to handle this.

The above error comes from Kernel::System::Main::Require(). In that sub the builtin require is called. require then throws the "Attempt to reload %s aborted" message because a broken module had been required before. See https://perldoc.perl.org/perldiag#Attempt-to-reload-%25s-aborted. How %INCtells requireabout the previous failed attempt is discussed in https://github.com/perl/perl5/issues/13797 . I could image that Kernel::System::Main::Require() could be enhanced to check for that case.

But if I understand refresh_module_if_modified(), https://metacpan.org/release/BPS/Module-Refresh-0.18/source/lib/Module/Refresh.pm#L96, correctly, then the same issue applies there too. No reloading is attempted when $INC{$mod} is false.

Interestingly $INC{$mod} is cleared in refresh_module, https://metacpan.org/release/BPS/Module-Refresh-0.18/source/lib/Module/Refresh.pm#L114 .

So, I propose to keep this issue on the wishlist until a good idea surfaces.

stefanhaerter commented 3 months ago

Out of curiosity: Kernel::System::Main::Require() checks for $@ - can we be sure that this variable is cleaned properly or would it be an option to clear it manually at the start of the sub?

bschmalhofer commented 3 months ago

Clearing $@ is done by the eval itself, see the 5th paragraph in https://perldoc.perl.org/functions/eval . But messing with a global variable is bad practice in almost any case. So a good change would be to use Try::Tiny instead. But this would not help with the builtin behavior that requiredoes not try to reload a module that failed already. See also #1695 .