| Summary: | Fix use of cronlogaction | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Magnus Enger <magnus> |
| Component: | Notices | Assignee: | Magnus Enger <magnus> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | jonathan.druart, lucas |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.06
|
|
| Circulation function: | |||
| Bug Depends on: | 31203 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 33649: Fix use of cronlogaction
Bug 33649: Fix use of cronlogaction Bug 33649: Fix use of cronlogaction |
||
There is an other one in misc/cronjobs/fines.pl. (In reply to Jonathan Druart from comment #1) > There is an other one in misc/cronjobs/fines.pl. Yeah, just spotted it. :-) Created attachment 150563 [details] [review] Bug 33649: Fix use of cronlogaction C4::Log::cronlogaction() takes a hashref as argument, with "info" and possibly "action" as keys. But there are a couple of places where it is called with just a string as argument, and that does not work. Both places need lock_exec to fail to trigger the error. I have seen this on a production server, but not been able to reproduce in ktd. To test: - Run this on the Koha repo: grep -r "cronlogaction(" * - Verify that fines.pl and process_message_queue.pl are the only scripts that call cronlogaction without a hashref as argument, but do it like this: cronlogaction( $message ); - Apply this patch - Run the grep again and verify that all calls to cronlogaction now take a hashref as argument Created attachment 150573 [details] [review] Bug 33649: Fix use of cronlogaction C4::Log::cronlogaction() takes a hashref as argument, with "info" and possibly "action" as keys. But there are a couple of places where it is called with just a string as argument, and that does not work. Both places need lock_exec to fail to trigger the error. I have seen this on a production server, but not been able to reproduce in ktd. To test: - Run this on the Koha repo: grep -r "cronlogaction(" * - Verify that fines.pl and process_message_queue.pl are the only scripts that call cronlogaction without a hashref as argument, but do it like this: cronlogaction( $message ); - Apply this patch - Run the grep again and verify that all calls to cronlogaction now take a hashref as argument Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 150577 [details] [review] Bug 33649: Fix use of cronlogaction C4::Log::cronlogaction() takes a hashref as argument, with "info" and possibly "action" as keys. But there are a couple of places where it is called with just a string as argument, and that does not work. Both places need lock_exec to fail to trigger the error. I have seen this on a production server, but not been able to reproduce in ktd. To test: - Run this on the Koha repo: grep -r "cronlogaction(" * - Verify that fines.pl and process_message_queue.pl are the only scripts that call cronlogaction without a hashref as argument, but do it like this: cronlogaction( $message ); - Apply this patch - Run the grep again and verify that all calls to cronlogaction now take a hashref as argument Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Can be tested easily adding a sleep right after the lock. Pushed to master for 23.05. Nice work everyone, thanks! Many hands makes light work, thankyou everyone! Pushed to 22.11.x for the next release Missing dependencies for 22.05.x, no backport. |
process_message_queue.pl has this code: try { $script_handler->lock_exec; } catch { my $message = "Skipping execution of $0 ($_)"; print STDERR "$message\n" if $verbose; cronlogaction( $message ); exit; }; If lock_exec fails for some reason, we get this error: $ sudo koha-foreach --chdir --enabled --email /usr/share/koha/bin/cronjobs/process_message_queue.pl Can't use string ("Skipping execution of /usr/share"...) as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Log.pm line 135. mykoha: 13 status returned by "/usr/share/koha/bin/cronjobs/process_message_queue.pl" This is because cronlogaction should be called like this: cronlogaction({ info => $message })