Bug 24313 - XSLT errors should show in the logs
Summary: XSLT errors should show in the logs
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 20272
Blocks:
  Show dependency treegraph
 
Reported: 2019-12-27 13:37 UTC by Katrin Fischer
Modified: 2021-06-14 21:28 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.03, 19.05.08, 18.11.14


Attachments
Bug 24313: Always display XSLT errors in logs (1.40 KB, patch)
2019-12-27 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24313: Always display XSLT errors in logs (1.40 KB, patch)
2019-12-27 15:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24313: Always display XSLT errors in logs (1.55 KB, patch)
2020-01-10 07:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2019-12-27 13:37:38 UTC
A broken XSLT file will just "silently" die and there are no longer any errors in the logs that can help to identify the problem. 

This used to work and must have changed somewhere sometime.
Comment 1 Jonathan Druart 2019-12-27 13:53:45 UTC
From
  commit 295ae33800a322facfdf56795f4c02b2fd53432b
  Bug 20272: Replace error numbers by codes in XSLT_Handler

"""
    Make XSLT_Handler a little bit less noisy by defaulting print_warns to
    false unless $ENV{DEBUG} is set. (See also bug 19018).
"""


Marcel, I think we should warn the errors in the logs. They are not debug messages here, we should not rely on DEBUG.
If too noisy we should fix the original issues, not hide all the errors (which make the XSLT debugging super hard)
Comment 2 Jonathan Druart 2019-12-27 13:57:04 UTC
Created attachment 96650 [details] [review]
Bug 24313: Always display XSLT errors in logs

From
  commit 295ae33800a322facfdf56795f4c02b2fd53432b
  Bug 20272: Replace error numbers by codes in XSLT_Handler

"""
    Make XSLT_Handler a little bit less noisy by defaulting print_warns to
    false unless $ENV{DEBUG} is set. (See also bug 19018).
"""

I think we should warn the errors in the logs. They are not debug messages here, we should not rely on DEBUG.
If too noisy we should fix the original issues, not hide all the errors (which make the XSLT debugging super hard)

Test plan:
Break a XSLT, reload the page and confirm that there are useful errors in the Koha log file
Comment 3 Katrin Fischer 2019-12-27 15:05:13 UTC
Created attachment 96654 [details] [review]
Bug 24313: Always display XSLT errors in logs

From
  commit 295ae33800a322facfdf56795f4c02b2fd53432b
  Bug 20272: Replace error numbers by codes in XSLT_Handler

"""
    Make XSLT_Handler a little bit less noisy by defaulting print_warns to
    false unless $ENV{DEBUG} is set. (See also bug 19018).
"""

I think we should warn the errors in the logs. They are not debug messages here, we should not rely on DEBUG.
If too noisy we should fix the original issues, not hide all the errors (which make the XSLT debugging super hard)

Test plan:
Break a XSLT, reload the page and confirm that there are useful errors in the Koha log file
Comment 4 Marcel de Rooy 2020-01-10 07:04:35 UTC
Created attachment 97146 [details] [review]
Bug 24313: Always display XSLT errors in logs

From
  commit 295ae33800a322facfdf56795f4c02b2fd53432b
  Bug 20272: Replace error numbers by codes in XSLT_Handler

"""
    Make XSLT_Handler a little bit less noisy by defaulting print_warns to
    false unless $ENV{DEBUG} is set. (See also bug 19018).
"""

I think we should warn the errors in the logs. They are not debug messages here, we should not rely on DEBUG.
If too noisy we should fix the original issues, not hide all the errors (which make the XSLT debugging super hard)

Test plan:
Break a XSLT, reload the page and confirm that there are useful errors in the Koha log file

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Martin Renvoize 2020-01-10 08:39:08 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 6 Joy Nelson 2020-01-30 23:33:38 UTC
Pushed to 19.11.x branch for 19.11.03
Comment 7 Lucas Gass 2020-02-05 21:38:08 UTC
backported to 19.05.x for 19.05.08
Comment 8 Hayley Pelham 2020-02-20 20:53:52 UTC
Backported to 18.11.x for 18.11.14