Bug 39911 - Fatal errors from SIP server are not logged
Summary: Fatal errors from SIP server are not logged
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Kyle M Hall (khall)
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-05-15 17:23 UTC by Kyle M Hall (khall)
Modified: 2025-07-01 21:21 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This restores the logging of fatal SIP errors to both the SIP logs and standard output from the command line.
Version(s) released in:
25.11.00,25.05.01
Circulation function:


Attachments
Bug 39911: Introduce failure in patron information request [DO NOT PUSH] (746 bytes, patch)
2025-05-15 17:30 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 39911: Trap DIE signals in SIP server (1.43 KB, patch)
2025-05-15 17:30 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 39911: Trap DIE signals in SIP server (1.47 KB, patch)
2025-05-15 17:31 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 39911: Trap DIE signals in SIP server (1.51 KB, patch)
2025-05-16 07:01 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 39911: Trap DIE signals in SIP server (1.56 KB, patch)
2025-06-06 18:20 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39911: Add POD (863 bytes, patch)
2025-06-12 12:44 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 39911: (RM follow-up) Use logger warn (813 bytes, patch)
2025-06-12 15:58 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 39911: Remove redundant warn (1.31 KB, patch)
2025-06-13 18:32 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2025-05-15 17:23:10 UTC
At some point our SIP server stopped logging fatal errors to either logs or standard error. We should restore this vital functionality.
Comment 1 Kyle M Hall (khall) 2025-05-15 17:30:01 UTC
Created attachment 182501 [details] [review]
Bug 39911: Introduce failure in patron information request [DO NOT PUSH]
Comment 2 Kyle M Hall (khall) 2025-05-15 17:30:23 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall (khall) 2025-05-15 17:31:53 UTC Comment hidden (obsolete)
Comment 4 Magnus Enger 2025-05-16 07:01:36 UTC
Created attachment 182508 [details] [review]
Bug 39911: Trap DIE signals in SIP server

At some point our SIP server stopped logging fatal errors to either logs or standard error. We should restore this vital functionality.

Test Plan:
1) Apply the first patch which will cause the patron information request
   to fail
2) Stop the SIP server
3) Start it again with: perl /kohadevbox/koha/C4/SIP/SIPServer.pm /etc/koha/sites/kohadev/SIPconfig.xml
4) Trigger the failure with the following command:
   ./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l MPL --patron koha -m patron_information
5) Note "THIS IS A TEST" is not found in logs or stderr
6) Apply the second patch
7) Repeat steps 2-4
8) Note the error message is now visible from stderr at the command line
   and also in the SIP log

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 5 Tomás Cohen Arazi (tcohen) 2025-06-06 18:20:21 UTC
Created attachment 183078 [details] [review]
Bug 39911: Trap DIE signals in SIP server

At some point our SIP server stopped logging fatal errors to either logs or standard error. We should restore this vital functionality.

Test Plan:
1) Apply the first patch which will cause the patron information request
   to fail
2) Stop the SIP server
3) Start it again with: perl /kohadevbox/koha/C4/SIP/SIPServer.pm /etc/koha/sites/kohadev/SIPconfig.xml
4) Trigger the failure with the following command:
   ./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l MPL --patron koha -m patron_information
5) Note "THIS IS A TEST" is not found in logs or stderr
6) Apply the second patch
7) Repeat steps 2-4
8) Note the error message is now visible from stderr at the command line
   and also in the SIP log

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Lucas Gass (lukeg) 2025-06-11 20:08:05 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 7 Lucas Gass (lukeg) 2025-06-11 21:53:07 UTC
This is causing tests to fail in main due to bad POD coverage.

xt/author/pod_coverage.t is failing
Comment 8 Kyle M Hall (khall) 2025-06-12 12:44:30 UTC
Created attachment 183203 [details] [review]
Bug 39911: Add POD
Comment 9 Lucas Gass (lukeg) 2025-06-12 15:56:42 UTC
I think its actually the bare 'warn' that is causing test to fail. I think we should use 

$logger->warn()
Comment 10 Lucas Gass (lukeg) 2025-06-12 15:58:28 UTC
Created attachment 183212 [details] [review]
Bug 39911: (RM follow-up) Use logger warn
Comment 11 Lucas Gass (lukeg) 2025-06-13 13:35:11 UTC
Tests are still failing here, even with Kyle's follow-up.
Comment 12 Tomás Cohen Arazi (tcohen) 2025-06-13 18:32:30 UTC
Created attachment 183247 [details] [review]
Bug 39911: Remove redundant warn

Following the original test plan you notice, `THIS IS A TEST` is printed
twice:

* In the logs.
* In STDERR for the SIP server.

The latter is redundant, and also problematic for
`xt/author/pod_coverage.t` which fails the `no warnings` tests because
of that.

To test:
1. Run:
   $ ktd --shell
  k$ prove xt/author/pod_coverage.t
=> FAIL: Scary errors, lots of them
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Follow the original bug test plan
=> SUCCESS: There's `THIS IS A TEST` in the logs, which was the author's
goal in the first place.
5. Sign off :-D
6. Visit bug 40144 :-P

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi (tcohen) 2025-06-13 18:33:21 UTC
(In reply to Lucas Gass (lukeg) from comment #11)
> Tests are still failing here, even with Kyle's follow-up.

Lucas, I checked the warn is not required to accomplish Kyle's goal so it is ok to push my follow-up.

Cheers.
Comment 14 Lucas Gass (lukeg) 2025-06-13 19:10:23 UTC
Follow-ups pushed to main, thank you!
Comment 15 Paul Derscheid 2025-06-22 21:47:03 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03