From 8604d5b9b8e20df520706b530ea1009194d281c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johanna=20R=C3=A4is=C3=A4?= Date: Tue, 27 Jan 2026 13:31:24 +0200 Subject: [PATCH] Bug 41707: Modify C4::SIP::Trapper DIE handling for better logging This patch improves the error handling in the SIP Trapper module by logging only SIP-related errors as errors and non-SIP errors as warnings. This change aims to reduce noise in the logs and make it easier to identify genuine SIP-related issues. Test plan: 1. Apply the patch 2. prove -lv t/db_dependent/SIP/Trapper.t 3. Ensure all tests pass without errors 4. Start the server: perl C4/SIP/SIPServer.pm /etc/koha/SIPconfig.xml 5. misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l MPL --patron koha -m patron_information 6. Verify that only SIP-related errors are logged as errors and non-SIP errors are warned. Sponsored-by: Koha-Suomi Oy --- C4/SIP/Trapper.pm | 22 ++++++++- t/db_dependent/SIP/Trapper.t | 95 ++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 2 deletions(-) create mode 100755 t/db_dependent/SIP/Trapper.t diff --git a/C4/SIP/Trapper.pm b/C4/SIP/Trapper.pm index 5254fba043..ecbd32da40 100644 --- a/C4/SIP/Trapper.pm +++ b/C4/SIP/Trapper.pm @@ -65,9 +65,27 @@ sub BINMODE { =cut $SIG{__DIE__} = sub { - my $msg = shift; + my $msg = shift; + + # Check if we are inside an eval + my $logger_method = 'error'; + $logger_method = 'warn' if defined $^S && $^S != 0; + + # Check if error originated from SIP code + my $is_sip_error = 0; + my $i = 0; # Start from 0 to check the current caller + while ( my @caller = caller( $i++ ) ) { + if ( $caller[0] =~ /^C4::SIP::/ || $caller[1] =~ /\/C4\/SIP\// ) { + $is_sip_error = 1; + last; + } + last if $i > 10; # Don't check too deep + } + + # Only log if it's from SIP code + return unless $is_sip_error; my $logger = Koha::Logger->get( { interface => 'sip', category => 'STDERR' } ); - $logger->error($msg) if $logger; + $logger->$logger_method($msg) if $logger; die $msg; }; diff --git a/t/db_dependent/SIP/Trapper.t b/t/db_dependent/SIP/Trapper.t new file mode 100755 index 0000000000..07b01132e7 --- /dev/null +++ b/t/db_dependent/SIP/Trapper.t @@ -0,0 +1,95 @@ +#!/usr/bin/perl + +use Modern::Perl; +use Test::More tests => 8; +use Test::MockModule; +use Test::NoWarnings; + +# Load SIP modules (this loads Trapper and sets up the DIE handler) +use_ok('C4::SIP::Trapper'); + +# Verify DIE handler is installed +ok( exists $SIG{__DIE__}, "DIE signal handler is installed" ); + +# Mock Koha::Logger to capture both warnings and errors +my @logged_warnings; +my @logged_errors; +my $mock_logger = Test::MockModule->new('Koha::Logger'); +$mock_logger->mock( + 'get', + sub { + my $self = bless {}, 'Koha::Logger'; + return $self; + } +); +$mock_logger->mock( + 'warn', + sub { + my ( $self, $msg ) = @_; + push @logged_warnings, $msg; + } +); +$mock_logger->mock( + 'error', + sub { + my ( $self, $msg ) = @_; + push @logged_errors, $msg; + } +); + +# Create a SIP module that will throw an error +{ + + package C4::SIP::TestError; + use Modern::Perl; + + sub cause_fatal_error { + die "Fatal error in SIP module\n"; + } +} + +# SIP errors inside eval should be logged as WARNING +# The DIE handler checks SIP namespace first, then checks if in eval and logs accordingly +@logged_warnings = (); +@logged_errors = (); +eval { C4::SIP::TestError::cause_fatal_error(); }; + +like( $@, qr/Fatal error in SIP module/, "SIP error was caught by eval" ); +ok( + scalar(@logged_warnings) > 0 && $logged_warnings[0] =~ /Fatal error in SIP module/, + "SIP error inside eval was logged as WARNING by Trapper" +); +ok( + scalar(@logged_errors) == 0, + "SIP error inside eval was NOT logged as error (only warning)" +); + +# Verify non-SIP errors are never logged +{ + + package C4::NOTSIP::TestError; + use Modern::Perl; + + sub cause_fatal_error { + die "Fatal error in NOTSIP module\n"; + } +} + +@logged_warnings = (); +@logged_errors = (); +eval { C4::NOTSIP::TestError::cause_fatal_error(); }; + +# Trapper now checks SIP namespace FIRST, then decides what to log +# Non-SIP errors return early and are never logged +ok( + scalar(@logged_warnings) == 0 && scalar(@logged_errors) == 0, + "Non-SIP error was NOT logged at all (Trapper filters by namespace first)" +); +like( $@, qr/Fatal error in NOTSIP module/, "Non-SIP error was still caught by eval" ); + +# NOTE: Errors outside eval cannot be tested in a test suite because: +# 1. Tests require eval to catch errors to prevent test termination +# 2. The DIE handler checks $^S to detect eval context +# 3. Outside eval, SIP errors would be logged as ERROR level and re-thrown +# To verify production behavior, check SIP logs for uncaught fatal errors + -- 2.34.1