Bug 29387

Summary: BatchUpdateBiblio does not handle exception correctly
Product: Koha Reporter: Jonathan Druart <jonathan.druart+koha>
Component: ToolsAssignee: Jonathan Druart <jonathan.druart+koha>
Status: Pushed to master --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: fridolin.somers, joonas.kylmala, kyle, m.de.rooy, nugged
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on: 22417    
Bug Blocks:    
Attachments: Bug 29387: Stringify exception when logging error during a batch mod biblio
Bug 29387: Stringify exception when logging error during a batch mod biblio
Bug 29387: Strinfigy exceptions for other background job modules
Bug 29387: Stringify exception when logging error during a batch mod biblio
Bug 29387: Strinfigy exceptions for other background job modules
Bug 29387: Stringify exception when logging error during a batch mod biblio
Bug 29387: Stringify exceptions for other background job modules
Bug 29387: (QA follow-up) Fix modules in test
Bug 29387: [21.05] Stringify exception when logging error during a batch mod biblio

Description Jonathan Druart 2021-11-02 09:42:54 UTC
If an exception is raised by one the methods/subroutines called when batch biblio update, the job will fail in the middle and some records won't be processed.
Comment 1 Jonathan Druart 2021-11-02 09:48:09 UTC
Created attachment 127205 [details] [review]
Bug 29387: Stringify exception when logging error during a batch mod biblio

If an exception is raised by one the methods/subroutines called when batch biblio update, the job will fail in the middle and some records won't be processed.

Test plan:
0. Don't apply the patch
In KTD you can run the a batch mod biblio on the 100 first records and
the worker will fail with
"""
countered object 'C4::Biblio::_koha_modify_biblioitem_nonmarc(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'lccn' at row 1 at /kohadevbox/koha/C4/Biblio.pm line 423
', but neither allow_blessed, convert_blessed nor allow_tags settings are enabled (or TO_JSON/FREEZE method missing) at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateBiblio.pm line 120.
"""
The UI is not saying anything about this problem.

Generate 100 biblionumbers: perl -e 'for (1..100){print "$_\n"}'
1. Apply the patch and confirm than now you see the detail of the batch
update and that biblionumber 72 is marked as not been modified with the
correct error (raw DBI error)
Comment 2 Jonathan Druart 2021-11-02 10:44:25 UTC
Created attachment 127207 [details] [review]
Bug 29387: Stringify exception when logging error during a batch mod biblio

If an exception is raised by one the methods/subroutines called when batch biblio update, the job will fail in the middle and some records won't be processed.

Test plan:
0. Don't apply the patch
In KTD you can run the a batch mod biblio on the 100 first records and
the worker will fail with
"""
countered object 'C4::Biblio::_koha_modify_biblioitem_nonmarc(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'lccn' at row 1 at /kohadevbox/koha/C4/Biblio.pm line 423
', but neither allow_blessed, convert_blessed nor allow_tags settings are enabled (or TO_JSON/FREEZE method missing) at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateBiblio.pm line 120.
"""
The UI is not saying anything about this problem.

Generate 100 biblionumbers: perl -e 'for (1..100){print "$_\n"}'
1. Apply the patch and confirm than now you see the detail of the batch
update and that biblionumber 72 is marked as not been modified with the
correct error (raw DBI error)
Comment 3 Joonas Kylmälä 2021-11-06 12:48:14 UTC
This only fixes the bug from one place, these need the same fix:

./Koha/BackgroundJob/BatchDeleteAuthority.pm60:                error => ($@ ? $@ : 0),
./Koha/BackgroundJob/BatchUpdateAuthority.pm93:                error => ($@ ? $@ : 0),
./Koha/BackgroundJob/BatchCancelHold.pm97:                ? ( $@ ? $@ : 0 )
./Koha/BackgroundJob/BatchDeleteBiblio.pm139:                error => ($@ ? $@ : $error),
Comment 4 Jonathan Druart 2021-11-08 09:23:40 UTC
Created attachment 127431 [details] [review]
Bug 29387: Strinfigy exceptions for other background job modules
Comment 5 Fridolin Somers 2021-11-12 07:11:08 UTC
We have two behaviors :
  error => "$@"
  error => ($@ ? "$@" : 0)
Shouldn't it be only one here ?
Comment 6 Joonas Kylmälä 2021-11-12 18:13:52 UTC
Created attachment 127594 [details] [review]
Bug 29387: Stringify exception when logging error during a batch mod biblio

If an exception is raised by one the methods/subroutines called when batch biblio update, the job will fail in the middle and some records won't be processed.

Test plan:
0. Don't apply the patch
In KTD you can run the a batch mod biblio on the 100 first records and
the worker will fail with
"""
countered object 'C4::Biblio::_koha_modify_biblioitem_nonmarc(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'lccn' at row 1 at /kohadevbox/koha/C4/Biblio.pm line 423
', but neither allow_blessed, convert_blessed nor allow_tags settings are enabled (or TO_JSON/FREEZE method missing) at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateBiblio.pm line 120.
"""
The UI is not saying anything about this problem.

Generate 100 biblionumbers: perl -e 'for (1..100){print "$_\n"}'
1. Apply the patch and confirm than now you see the detail of the batch
update and that biblionumber 72 is marked as not been modified with the
correct error (raw DBI error)

JK: add executable bit to BatchUpdateBiblio.t
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 7 Joonas Kylmälä 2021-11-12 18:13:56 UTC
Created attachment 127595 [details] [review]
Bug 29387: Strinfigy exceptions for other background job modules

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 8 Joonas Kylmälä 2021-11-12 18:16:29 UTC
(In reply to Fridolin Somers from comment #5)
> We have two behaviors :
>   error => "$@"
>   error => ($@ ? "$@" : 0)
> Shouldn't it be only one here ?

In the   error => "$@" cases there is already the if else check in the outer "if else" block so no need for this.
Comment 9 Marcel de Rooy 2021-11-19 07:20:46 UTC
QAing
Comment 10 Marcel de Rooy 2021-11-19 07:41:21 UTC
Did we consider the allow_blessed/convert_blessed route in JSON?
Comment 11 Marcel de Rooy 2021-11-19 07:43:36 UTC
Created attachment 127825 [details] [review]
Bug 29387: Stringify exception when logging error during a batch mod biblio

If an exception is raised by one the methods/subroutines called when batch biblio update, the job will fail in the middle and some records won't be processed.

Test plan:
0. Don't apply the patch
In KTD you can run the a batch mod biblio on the 100 first records and
the worker will fail with
"""
countered object 'C4::Biblio::_koha_modify_biblioitem_nonmarc(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'lccn' at row 1 at /kohadevbox/koha/C4/Biblio.pm line 423
', but neither allow_blessed, convert_blessed nor allow_tags settings are enabled (or TO_JSON/FREEZE method missing) at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateBiblio.pm line 120.
"""
The UI is not saying anything about this problem.

Generate 100 biblionumbers: perl -e 'for (1..100){print "$_\n"}'
1. Apply the patch and confirm than now you see the detail of the batch
update and that biblionumber 72 is marked as not been modified with the
correct error (raw DBI error)

JK: add executable bit to BatchUpdateBiblio.t
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2021-11-19 07:43:39 UTC
Created attachment 127826 [details] [review]
Bug 29387: Stringify exceptions for other background job modules

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2021-11-19 07:43:43 UTC
Created attachment 127827 [details] [review]
Bug 29387: (QA follow-up) Fix modules in test

Can't locate object method "new" via package "Koha::BackgroundJob::BatchUpdateBiblio" (perhaps you forgot to load "Koha::BackgroundJob::BatchUpdateBiblio"?) at t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t line 45.

How did it pass at signoff?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Jonathan Druart 2021-11-19 09:41:46 UTC
(In reply to Marcel de Rooy from comment #13)
> Created attachment 127827 [details] [review] [review]
> Bug 29387: (QA follow-up) Fix modules in test
> 
> Can't locate object method "new" via package
> "Koha::BackgroundJob::BatchUpdateBiblio" (perhaps you forgot to load
> "Koha::BackgroundJob::BatchUpdateBiblio"?) at
> t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t line 45.
> 
> How did it pass at signoff?
> 
> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Thanks for the follow-up. It got broken by bug 28413 that was pushed in the meanwhile.

(In reply to Marcel de Rooy from comment #10)
> Did we consider the allow_blessed/convert_blessed route in JSON?

I didn't, we could but I wouldn't introduce such changes for 21.11.00
Comment 15 Jonathan Druart 2021-11-19 14:15:42 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 16 Kyle M Hall 2021-11-19 16:57:30 UTC
Doesn't apply to 21.05.x. Please rebase and let me know if this is needed in 21.05!
Comment 17 Jonathan Druart 2021-11-22 10:53:47 UTC
Created attachment 127924 [details] [review]
Bug 29387: [21.05] Stringify exception when logging error during a batch mod biblio

If an exception is raised by one the methods/subroutines called when batch biblio update, the job will fail in the middle and some records won't be processed.

Test plan:
0. Don't apply the patch
In KTD you can run the a batch mod biblio on the 100 first records and
the worker will fail with
"""
countered object 'C4::Biblio::_koha_modify_biblioitem_nonmarc(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'lccn' at row 1 at /kohadevbox/koha/C4/Biblio.pm line 423
', but neither allow_blessed, convert_blessed nor allow_tags settings are enabled (or TO_JSON/FREEZE method missing) at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateBiblio.pm line 120.
"""
The UI is not saying anything about this problem.

Generate 100 biblionumbers: perl -e 'for (1..100){print "$_\n"}'
1. Apply the patch and confirm than now you see the detail of the batch
update and that biblionumber 72 is marked as not been modified with the
correct error (raw DBI error)

JK: add executable bit to BatchUpdateBiblio.t
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 29387: Stringify exceptions for other background job modules

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 29387: (QA follow-up) Fix modules in test

Can't locate object method "new" via package "Koha::BackgroundJob::BatchUpdateBiblio" (perhaps you forgot to load "Koha::BackgroundJob::BatchUpdateBiblio"?) at t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t line 45.

How did it pass at signoff?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Jonathan Druart 2021-11-22 10:54:50 UTC
(In reply to Kyle M Hall from comment #16)
> Doesn't apply to 21.05.x. Please rebase and let me know if this is needed in
> 21.05!

Yes it's needed, I rebased and squashed the patches.