Bug 23425

Summary: Search explodes with "invalid data, cannot decode object"
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: SearchingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: major    
Priority: P5 - low CC: fridolin.somers, katrin.fischer, lucas, rudolfbyker, tomascohen
Version: master   
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:
19.11.00,19.05.04
Bug Depends on: 22144    
Bug Blocks: 23846, 24574    
Attachments: Bug 23425: Display the MARCXML error in the Metadata::Invalid exception message
Bug 23425: Display the MARCXML error in the Metadata::Invalid exception message
Bug 23425: (QA follow-up) Keep it generic

Description Jonathan Druart 2019-08-04 15:32:45 UTC
If the MARCXML data is wrong for some reasons and so MARC::Record::new_from_xml fails, a search will explodes with "invalid data, cannot decode object"

We should:
- Understand why the data is wrong
- Provide a maintenance script to catch problematic data
- Skip the record instead of exploding (or maybe not, it will hide a data problem)

It has been highlighted by bug 22144, was silently failing before those patches.
Comment 1 Jonathan Druart 2019-08-04 15:37:19 UTC
Also, Koha::Exceptions::Metadata should include the reason of the original error.
Comment 2 Jonathan Druart 2019-08-04 15:48:19 UTC
Created attachment 91948 [details] [review]
Bug 23425: Display the MARCXML error in the Metadata::Invalid exception message
Comment 3 Jonathan Druart 2019-08-04 15:49:58 UTC
(In reply to Jonathan Druart from comment #2)
> Created attachment 91948 [details] [review] [review]
> Bug 23425: Display the MARCXML error in the Metadata::Invalid exception
> message

Please apply this patch if you face this problem, then you should get more information about the root of the problem.
Comment 4 Tomás Cohen Arazi 2019-08-04 18:38:20 UTC
(In reply to Jonathan Druart from comment #0)
> If the MARCXML data is wrong for some reasons and so
> MARC::Record::new_from_xml fails, a search will explodes with "invalid data,
> cannot decode object"
> 
> We should:
> - Understand why the data is wrong
> - Provide a maintenance script to catch problematic data
> - Skip the record instead of exploding (or maybe not, it will hide a data
> problem)
> 
> It has been highlighted by bug 22144, was silently failing before those
> patches.

Do you think we should move this one forward into master?
Comment 5 Rudolf Byker 2019-08-05 07:51:54 UTC
Adding the Marc XML error is great, but there is still no way to fix the marc record from the web interface when this happens. I'm not advocating that the error be hidden (bad), but the "exploding" as you call it is worse. The correct way is to tell the user what went wrong, without breaking the UI, and giving them a way to fix the data in the UI.
Comment 6 Jonathan Druart 2019-08-05 13:11:19 UTC
(In reply to Rudolf Byker from comment #5)
> Adding the Marc XML error is great, but there is still no way to fix the
> marc record from the web interface when this happens. I'm not advocating
> that the error be hidden (bad), but the "exploding" as you call it is worse.
> The correct way is to tell the user what went wrong, without breaking the
> UI, and giving them a way to fix the data in the UI.

I wanted first to understand why the data was wrong (encoding, badly formatted, etc.) That's why I have attached a "debug patch".
Then I will provide a more global fix.
Comment 7 Tomás Cohen Arazi 2019-09-17 14:53:59 UTC
(In reply to Rudolf Byker from comment #5)
> Adding the Marc XML error is great, but there is still no way to fix the
> marc record from the web interface when this happens. I'm not advocating
> that the error be hidden (bad), but the "exploding" as you call it is worse.
> The correct way is to tell the user what went wrong, without breaking the
> UI, and giving them a way to fix the data in the UI.

One thing doesn't exclude the other. We need the information about the nature of the problem in order to provide a fix/workaround/recommendation.

I will try to move this patch into the stable branches, and keep an eye on the feedback.
Comment 8 Tomás Cohen Arazi 2019-09-17 18:09:22 UTC
Created attachment 92906 [details] [review]
Bug 23425: Display the MARCXML error in the Metadata::Invalid exception message

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2019-09-17 18:09:26 UTC
Created attachment 92907 [details] [review]
Bug 23425: (QA follow-up) Keep it generic

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Tomás Cohen Arazi 2019-09-20 17:10:47 UTC
Skipping the QA step.

@RM: Feel free to revert the status, I just think this is urgent and very simple.
Comment 11 Martin Renvoize 2019-09-23 09:44:22 UTC
Nice work!

Pushed to master for 19.11.00
Comment 12 Fridolin Somers 2019-09-24 06:55:22 UTC
Pushed to 19.05.x for 19.05.04
Comment 13 Lucas Gass 2019-10-17 19:33:11 UTC
missing dependency for 18.11.x, no backport
Comment 14 Rudolf Byker 2020-01-15 07:36:09 UTC
Some feedback from my side after upgrading to 19.11:

There is an error with this bibliographic record, the view may be degraded.
Error: Invalid data, cannot decode object (id=32057, format=marcxml, schema=MARC21, decoding_error=':27: parser error : PCDATA invalid Char value 27 <subfield code="a">Kp3s�benhavn :</subfield> ^ :27: parser error : PCDATA invalid Char value 27 <subfield code="a">Kp3s�benhavn :</subfield> ^') 

So it looks like it has to do with how the ø in København is encoded. The book is from Denmark.

Running "SELECT metadata FROM `biblio_metadata` where biblionumber=39335" reveals a MARC record that says it's encoded in UTF-8, but $260a is definitely in something other than UTF-8.

With Koha 19.11, I could simply click "Edit" and re-type the problematic field, and everything works well from there. Thanks.
Comment 15 Rudolf Byker 2020-02-03 07:41:25 UTC
Although this is fixed on the /cgi-bin/koha/catalogue/detail.pl page, it's still not handled gracefully on these pages:

/cgi-bin/koha/catalogue/ISBDdetail.pl
/cgi-bin/koha/catalogue/MARCdetail.pl

New issue? Or re-open this one?
Comment 16 Katrin Fischer 2020-02-03 08:41:59 UTC
(In reply to Rudolf Byker from comment #15)
> Although this is fixed on the /cgi-bin/koha/catalogue/detail.pl page, it's
> still not handled gracefully on these pages:
> 
> /cgi-bin/koha/catalogue/ISBDdetail.pl
> /cgi-bin/koha/catalogue/MARCdetail.pl
> 
> New issue? Or re-open this one?

Hi Rudolf, when in doubt always open a new bug report. Especially when the bug has been already pushed/released/closed as is the case here.