Bugzilla – Attachment 192093 Details for
Bug 41707
The DIE handler in C4::SIP::Trapper is logging non-SIP errors, causing log flooding.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41707: Modify C4::SIP::Trapper DIE handling for better logging
0001-Bug-41707-Modify-C4-SIP-Trapper-DIE-handling-for-bet.patch (text/plain), 4.93 KB, created by
Johanna Räisä
on 2026-01-27 11:50:13 UTC
(
hide
)
Description:
Bug 41707: Modify C4::SIP::Trapper DIE handling for better logging
Filename:
MIME Type:
Creator:
Johanna Räisä
Created:
2026-01-27 11:50:13 UTC
Size:
4.93 KB
patch
obsolete
>From 8604d5b9b8e20df520706b530ea1009194d281c6 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Johanna=20R=C3=A4is=C3=A4?= <johanna.raisa@gmail.com> >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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41707
: 192093