I have been noticing something while testing notices lately that I wanted to point out and see if anyone else can recreate, this is happening for me in KTD: To recreate: 1. Go to any notice and create some invalid Template Toolkit: [% IF ( 1 == 1 %] test [% END %] 2. Save and continue 3. Notice the Template Toolkit error: This is good. 3. Correct the error: [% IF ( 1 == 1 ) %] test [% END %] 4. Now save again. This time I see the Template Toolkit error like: DBIx::Class::Storage::DBI::mysql::_exec_svp_rollback(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist at /kohadevbox/koha/C4/Letters.pm line 1624 5. Save anyways 6. Logout of Koha and attempt to log back in: It explodes with this error: {UNKNOWN}: Transaction aborted: DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_4 does not exist at /kohadevbox/koha/Koha/Object.pm line 170 . Rollback failed: DBIx::Class::Storage::DBI::mysql::_exec_svp_rollback(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_3 does not exist at /kohadevbox/koha/Koha/Patron.pm line 363 at /kohadevbox/koha/Koha/Patron.pm line 363 at /usr/share/perl5/DBIx/Class/Exception.pm line 77
The issue seems to be combination of the try/catch (bug 31211) with the nested transaction (bug 28739) - when we fail we never rollback the transaction
Created attachment 144564 [details] [review] Bug 32442: Add tests
Created attachment 144565 [details] [review] Bug 32442: (bug 28739 follow-up) Ensure txn is rollbacked If the TT process call is returning false and so the croak is raised, then the txn is not rollbacked and the txn is not commited either. We need to ensure the txn will be correctly rollbacked. Test plan: 1. Go to any notice and create some invalid Template Toolkit: [% IF ( 1 == 1 %] test [% END %] 2. Save and continue 3. Logout of Koha and attempt to log back in => Without this patch you got Transaction aborted: DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_4 does not exist at /kohadevbox/koha/Koha/Object.pm line 170 . Rollback failed: DBIx::Class::Storage::DBI::mysql::_exec_svp_rollback(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_3 does not exist at /kohadevbox/koha/Koha/Patron.pm line 363 at /kohadevbox/koha/Koha/Patron.pm line 363 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 => With this patch applied the login works successfully QA Note: The test is not testing that the txn is rolledback, I didn't manage to test that.
QA: Looking here
Applied both patches, ran the test. Passed. Ran the test again: # Subtest: regression tests 1..8 DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Duplicate entry 'a_t_barcode' for key 'itembarcodeidx' at /usr/share/koha/Koha/Object.pm line 170 # No tests run! not ok 25 - No tests run for subtest "regression tests" # Failed test 'No tests run for subtest "regression tests"' # at t/db_dependent/Letters/TemplateToolkit.t line 1045. Checked my data: Lots of garbage items from the unit test in my database. This is not production ready yet :)
Problem introduced by this patch?
Created attachment 144686 [details] [review] Bug 32442: Add tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 144687 [details] [review] Bug 32442: (bug 28739 follow-up) Ensure txn is rollbacked If the TT process call is returning false and so the croak is raised, then the txn is not rollbacked and the txn is not commited either. We need to ensure the txn will be correctly rollbacked. Test plan: 1. Go to any notice and create some invalid Template Toolkit: [% IF ( 1 == 1 %] test [% END %] 2. Save and continue 3. Logout of Koha and attempt to log back in => Without this patch you got Transaction aborted: DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_4 does not exist at /kohadevbox/koha/Koha/Object.pm line 170 . Rollback failed: DBIx::Class::Storage::DBI::mysql::_exec_svp_rollback(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_3 does not exist at /kohadevbox/koha/Koha/Patron.pm line 363 at /kohadevbox/koha/Koha/Patron.pm line 363 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 => With this patch applied the login works successfully QA Note: The test is not testing that the txn is rolledback, I didn't manage to test that. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Marcel de Rooy from comment #5) > This is not production ready yet :) I was able to run multiple times with no problems - the test plan in staff interface worked as well
Running the test adds barcodes to the table like: +---------------------+ | barcode | +---------------------+ | a_t_barcode | | another_t_barcode | | another_t_barcode_3 | | IROjihTVYw1IMlUDV4 | | msuHU5tofEXn | | NMhOFzbI7Yjf | | oTfqL7AeaoSq9tDTJWQ | | Pu2W1UbP7HAvqZkum | | sblzCdKniw_bfJubrZA | | SXpFbuhE5TtTTq | | twTvJHKIkgDCZqh9R | | U8T51kYicLV4_JP | | UmNgCvommVa | | vf_sVPEcyM861c9 | | wY_QRsZyUSpFc | +---------------------+ This might not be the direct cause of this patch. But we should fix it here now where we are also dealing with the rollback etc.
(In reply to Marcel de Rooy from comment #10) > Running the test adds barcodes to the table like: > > +---------------------+ > | barcode | > +---------------------+ > | a_t_barcode | > | another_t_barcode | > | another_t_barcode_3 | > | IROjihTVYw1IMlUDV4 | > | msuHU5tofEXn | > | NMhOFzbI7Yjf | > | oTfqL7AeaoSq9tDTJWQ | > | Pu2W1UbP7HAvqZkum | > | sblzCdKniw_bfJubrZA | > | SXpFbuhE5TtTTq | > | twTvJHKIkgDCZqh9R | > | U8T51kYicLV4_JP | > | UmNgCvommVa | > | vf_sVPEcyM861c9 | > | wY_QRsZyUSpFc | > +---------------------+ > > This might not be the direct cause of this patch. But we should fix it here > now where we are also dealing with the rollback etc. I don't recreate and, if true, I don't think it should be done on this bug report. That sounds something different. select count(*) from items; # 961 prove t/db_dependent/Letters/TemplateToolkit.t t/db_dependent/Letters/TemplateToolkit.t .. ok All tests successful. select count(*) from items; # 961
Running prove with test gave no additional items for me too. Running the script from the commandline itself does: | a_t_barcode | | another_t_barcode | | another_t_barcode_3 | | BMFPNewTkRQ7 | | CSNGMjPP6jg | | XdvEtgyDT2vFo9xLw |
It is probably the LOCK in SendCirculationAlert..
Created attachment 145862 [details] [review] Bug 32442: Add tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 145863 [details] [review] Bug 32442: (bug 28739 follow-up) Ensure txn is rollbacked If the TT process call is returning false and so the croak is raised, then the txn is not rollbacked and the txn is not commited either. We need to ensure the txn will be correctly rollbacked. Test plan: 1. Go to any notice and create some invalid Template Toolkit: [% IF ( 1 == 1 %] test [% END %] 2. Save and continue 3. Logout of Koha and attempt to log back in => Without this patch you got Transaction aborted: DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_4 does not exist at /kohadevbox/koha/Koha/Object.pm line 170 . Rollback failed: DBIx::Class::Storage::DBI::mysql::_exec_svp_rollback(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_3 does not exist at /kohadevbox/koha/Koha/Patron.pm line 363 at /kohadevbox/koha/Koha/Patron.pm line 363 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 => With this patch applied the login works successfully QA Note: The test is not testing that the txn is rolledback, I didn't manage to test that. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
Not needed in 22.05.x, no backport