[Bug 42391] New: Use DBIx::Class exception_action to translate exceptions at the source
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Bug ID: 42391 Summary: Use DBIx::Class exception_action to translate exceptions at the source Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 19871 Bug 19871 introduced Koha::Schema::Util::ExceptionTranslator to centralize the translation of DBIx::Class exceptions into Koha exceptions. Currently, each caller (Object::store, Object::delete, Item::add_to_bundle) must explicitly call translate_exception in its catch block. DBIx::Class provides an exception_action hook on the Schema that intercepts every throw_exception call before it becomes a DBIx::Class::Exception. By setting this hook on Koha::Schema, we could translate MySQL errors into Koha exceptions at the source, eliminating the need for catch-block boilerplate in every caller. This would: - Remove the translate_exception catch blocks from Object::store, Object::delete, and Item::add_to_bundle - Make every DBIC operation (including raw resultset calls) automatically produce Koha exceptions without caller intervention - Close the gap where unmatched DBIx::Class::Exception errors could slip through template error handlers (identified during QA of bug 19871) - Refactor ExceptionTranslator to work on plain error strings instead of exception objects (exception_action receives the message before wrapping) The exception_action hook is available in DBIx::Class 0.082844 (shipped with KTD) and has been a core feature since early DBIC versions. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19871 [Bug 19871] Use new exceptions Koha::Exceptions::Object::DuplicateID and FKConstraint -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Use DBIx::Class |Use DBIx::Class |exception_action to |`exception_action` to |translate exceptions at the |translate exceptions at the |source |source -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com, | |kyle@bywatersolutions.com, | |martin.renvoize@openfifth.c | |o.uk Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197460 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197460&action=edit Bug 42391: Use exception_action to translate DBIC exceptions at the source DBIx::Class provides an exception_action hook that intercepts every throw_exception call before it becomes a DBIx::Class::Exception. By installing this hook on Koha::Schema via a connection() override, all database exceptions are automatically translated into Koha exceptions at the source. This eliminates the need for translate_exception catch blocks in every caller: - Remove try/catch from Koha::Object::store - Remove try/catch from Koha::Object::delete - Remove try/catch from Koha::Item::add_to_bundle - Remove translate_exception convenience method from Koha::Schema - Remove safe_do (already removed in previous follow-up) ExceptionTranslator is refactored to work on plain error message strings instead of DBIx::Class::Exception objects, since exception_action receives the message before wrapping. Unmatched errors fall through to DBIx::Class::Exception->throw() as the default, preserving backward compatibility. The warn $_->{msg} that was in Object::store is no longer needed since exceptions are translated before they propagate. Tests that asserted on that warning (via warning_like) are updated to use throws_ok directly. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Object.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Item.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Patron/Modifications.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197461 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197461&action=edit Bug 42391: (follow-up) Fix Objects.t case 3 to test FK deletion properly The 'Delete a patron that cannot be deleted' test (case 3) was using a stale $r_us object from case 2 (not in storage), so it was not actually testing FK constraint deletion. The assertion on DBIx::Class::Exception passed by accident. This follow-up: - Fetches a fresh Koha::Patron object for case 3 - Clears $e_us/$e_them before the new test - Asserts Koha::Exceptions::Object::FKConstraintDeletion, which is what exception_action now produces for FK deletion errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197462 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197462&action=edit Bug 42391: (follow-up) Add NotNull, NotInStorage and UnhandledDBError translations Extend the ExceptionTranslator to handle two additional MySQL error patterns: - NOT NULL violations ('Column X cannot be null' and 'X doesn't have a default value') → Koha::Exceptions::Object::NotNull - Not in database errors → Koha::Exceptions::Object::NotInStorage Add Koha::Exceptions::Object::NotNull exception class with a property field identifying the offending column. Add Koha::Exceptions::Object::UnhandledDBError as a catch-all for DBI errors that don't match any known pattern. This closes the gap where unmatched MySQL errors could leak as DBIx::Class::Exception to callers (e.g. templates that no longer catch that type). The exception_action handler now distinguishes three cases: - Koha exception objects (from txn_do re-throws): passed through via 'die $msg' to preserve the original exception type. Without this, txn_do's rollback-and-rethrow would stringify the Koha exception and wrap it in UnhandledDBError, losing the specific type. - DBI errors (prefixed 'DBI Exception:'): translated if possible, otherwise wrapped in UnhandledDBError. - DBIC internal errors (e.g. 'Not in database'): translated if possible, otherwise thrown as DBIx::Class::Exception. Note: NotInStorage translation works for update operations but not for delete-on-detached-Row where DBIC bypasses the Schema's exception_action. Those cases remain DBIx::Class::Exception. Update tests: - OAIServers.t, Z3950Servers.t: assert NotNull with property check - Hold.t: assert NotNull, remove warn suppression hack - Objects.t: update-not-in-storage cases assert NotInStorage - Upload.t: delete-not-in-storage stays DBIx::Class::Exception -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197522 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197522&action=edit Bug 42391: (follow-up) Rename ExceptionTranslator to ExceptionMapper The Koha::Schema::Util:: namespace adds unnecessary nesting for a single-purpose class. This follow-up moves it to Koha::Schema::ExceptionMapper, which sits alongside Koha::Schema itself — a better fit given it's tightly coupled to the Schema's exception_action hook. Changes: - Koha::Schema::Util::ExceptionTranslator → Koha::Schema::ExceptionMapper - Update Koha::Schema to use the new module - Move test file accordingly - Remove now-empty Koha/Schema/Util/ directory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197460|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197461|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197462|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197522|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 197731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197731&action=edit Bug 42391: Use exception_action to translate DBIC exceptions at the source DBIx::Class provides an exception_action hook that intercepts every throw_exception call before it becomes a DBIx::Class::Exception. By installing this hook on Koha::Schema via a connection() override, all database exceptions are automatically translated into Koha exceptions at the source. This eliminates the need for translate_exception catch blocks in every caller: - Remove try/catch from Koha::Object::store - Remove try/catch from Koha::Object::delete - Remove try/catch from Koha::Item::add_to_bundle - Remove translate_exception convenience method from Koha::Schema - Remove safe_do (already removed in previous follow-up) ExceptionTranslator is refactored to work on plain error message strings instead of DBIx::Class::Exception objects, since exception_action receives the message before wrapping. Unmatched errors fall through to DBIx::Class::Exception->throw() as the default, preserving backward compatibility. The warn $_->{msg} that was in Object::store is no longer needed since exceptions are translated before they propagate. Tests that asserted on that warning (via warning_like) are updated to use throws_ok directly. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Object.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Item.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Patron/Modifications.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 197732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197732&action=edit Bug 42391: (follow-up) Fix Objects.t case 3 to test FK deletion properly The 'Delete a patron that cannot be deleted' test (case 3) was using a stale $r_us object from case 2 (not in storage), so it was not actually testing FK constraint deletion. The assertion on DBIx::Class::Exception passed by accident. This follow-up: - Fetches a fresh Koha::Patron object for case 3 - Clears $e_us/$e_them before the new test - Asserts Koha::Exceptions::Object::FKConstraintDeletion, which is what exception_action now produces for FK deletion errors Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 197733 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197733&action=edit Bug 42391: (follow-up) Add NotNull, NotInStorage and UnhandledDBError translations Extend the ExceptionTranslator to handle two additional MySQL error patterns: - NOT NULL violations ('Column X cannot be null' and 'X doesn't have a default value') → Koha::Exceptions::Object::NotNull - Not in database errors → Koha::Exceptions::Object::NotInStorage Add Koha::Exceptions::Object::NotNull exception class with a property field identifying the offending column. Add Koha::Exceptions::Object::UnhandledDBError as a catch-all for DBI errors that don't match any known pattern. This closes the gap where unmatched MySQL errors could leak as DBIx::Class::Exception to callers (e.g. templates that no longer catch that type). The exception_action handler now distinguishes three cases: - Koha exception objects (from txn_do re-throws): passed through via 'die $msg' to preserve the original exception type. Without this, txn_do's rollback-and-rethrow would stringify the Koha exception and wrap it in UnhandledDBError, losing the specific type. - DBI errors (prefixed 'DBI Exception:'): translated if possible, otherwise wrapped in UnhandledDBError. - DBIC internal errors (e.g. 'Not in database'): translated if possible, otherwise thrown as DBIx::Class::Exception. Note: NotInStorage translation works for update operations but not for delete-on-detached-Row where DBIC bypasses the Schema's exception_action. Those cases remain DBIx::Class::Exception. Update tests: - OAIServers.t, Z3950Servers.t: assert NotNull with property check - Hold.t: assert NotNull, remove warn suppression hack - Objects.t: update-not-in-storage cases assert NotInStorage - Upload.t: delete-not-in-storage stays DBIx::Class::Exception Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 197734 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197734&action=edit Bug 42391: (follow-up) Rename ExceptionTranslator to ExceptionMapper The Koha::Schema::Util:: namespace adds unnecessary nesting for a single-purpose class. This follow-up moves it to Koha::Schema::ExceptionMapper, which sits alongside Koha::Schema itself — a better fit given it's tightly coupled to the Schema's exception_action hook. Changes: - Koha::Schema::Util::ExceptionTranslator → Koha::Schema::ExceptionMapper - Update Koha::Schema to use the new module - Move test file accordingly - Remove now-empty Koha/Schema/Util/ directory Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #9 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. QA output for running tests: Running tests (6) * Proving /kohadevbox/koha/t/db_dependent/Koha/Hold.t PASS * Proving /kohadevbox/koha/t/db_dependent/Koha/OAIServers.t PASS * Proving /kohadevbox/koha/t/db_dependent/Koha/Object.t PASS * Proving /kohadevbox/koha/t/db_dependent/Koha/Objects.t PASS Use of uninitialized value $file in concatenation (.) or string at /kohadevbox/qa-test-tools/koha-qa.pl line 347. * Proving Use of uninitialized value $file in concatenation (.) or string at /kohadevbox/qa-test-tools/koha-qa.pl line 348. PASS * Proving /kohadevbox/koha/t/db_dependent/Koha/Z3950Servers.t PASS 2. Individual tests pass: prove t/db_dependent/Koha/Object.t prove t/db_dependent/Koha/Objects.t prove t/db_dependent/Koha/Hold.t prove t/db_dependent/Koha/OAIServers.t prove t/db_dependent/Koha/Z3950Servers.t 3. No longer exists: t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #10 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198617&action=edit Bug 42391: (QA follow-up): Remove unused Try::Tiny from Koha::Item Patch from commit 2cf267a -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #11 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198618 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198618&action=edit Bug 42391: (QA follow-up): Clarify exception_action ref-passthrough comment Patch from commit e975454 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #12 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198619&action=edit Bug 42391: (QA follow-up): Fix unit tests Patch from commit a80446d -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197731|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197732|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197733|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197734|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198617|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198618|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198619|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #13 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198620&action=edit Bug 42391: Use exception_action to translate DBIC exceptions at the source DBIx::Class provides an exception_action hook that intercepts every throw_exception call before it becomes a DBIx::Class::Exception. By installing this hook on Koha::Schema via a connection() override, all database exceptions are automatically translated into Koha exceptions at the source. This eliminates the need for translate_exception catch blocks in every caller: - Remove try/catch from Koha::Object::store - Remove try/catch from Koha::Object::delete - Remove try/catch from Koha::Item::add_to_bundle - Remove translate_exception convenience method from Koha::Schema - Remove safe_do (already removed in previous follow-up) ExceptionTranslator is refactored to work on plain error message strings instead of DBIx::Class::Exception objects, since exception_action receives the message before wrapping. Unmatched errors fall through to DBIx::Class::Exception->throw() as the default, preserving backward compatibility. The warn $_->{msg} that was in Object::store is no longer needed since exceptions are translated before they propagate. Tests that asserted on that warning (via warning_like) are updated to use throws_ok directly. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Object.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Item.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Patron/Modifications.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #14 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198621 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198621&action=edit Bug 42391: (follow-up) Fix Objects.t case 3 to test FK deletion properly The 'Delete a patron that cannot be deleted' test (case 3) was using a stale $r_us object from case 2 (not in storage), so it was not actually testing FK constraint deletion. The assertion on DBIx::Class::Exception passed by accident. This follow-up: - Fetches a fresh Koha::Patron object for case 3 - Clears $e_us/$e_them before the new test - Asserts Koha::Exceptions::Object::FKConstraintDeletion, which is what exception_action now produces for FK deletion errors Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #15 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198622 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198622&action=edit Bug 42391: (follow-up) Add NotNull, NotInStorage and UnhandledDBError translations Extend the ExceptionTranslator to handle two additional MySQL error patterns: - NOT NULL violations ('Column X cannot be null' and 'X doesn't have a default value') → Koha::Exceptions::Object::NotNull - Not in database errors → Koha::Exceptions::Object::NotInStorage Add Koha::Exceptions::Object::NotNull exception class with a property field identifying the offending column. Add Koha::Exceptions::Object::UnhandledDBError as a catch-all for DBI errors that don't match any known pattern. This closes the gap where unmatched MySQL errors could leak as DBIx::Class::Exception to callers (e.g. templates that no longer catch that type). The exception_action handler now distinguishes three cases: - Koha exception objects (from txn_do re-throws): passed through via 'die $msg' to preserve the original exception type. Without this, txn_do's rollback-and-rethrow would stringify the Koha exception and wrap it in UnhandledDBError, losing the specific type. - DBI errors (prefixed 'DBI Exception:'): translated if possible, otherwise wrapped in UnhandledDBError. - DBIC internal errors (e.g. 'Not in database'): translated if possible, otherwise thrown as DBIx::Class::Exception. Note: NotInStorage translation works for update operations but not for delete-on-detached-Row where DBIC bypasses the Schema's exception_action. Those cases remain DBIx::Class::Exception. Update tests: - OAIServers.t, Z3950Servers.t: assert NotNull with property check - Hold.t: assert NotNull, remove warn suppression hack - Objects.t: update-not-in-storage cases assert NotInStorage - Upload.t: delete-not-in-storage stays DBIx::Class::Exception Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #16 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198623 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198623&action=edit Bug 42391: (follow-up) Rename ExceptionTranslator to ExceptionMapper The Koha::Schema::Util:: namespace adds unnecessary nesting for a single-purpose class. This follow-up moves it to Koha::Schema::ExceptionMapper, which sits alongside Koha::Schema itself — a better fit given it's tightly coupled to the Schema's exception_action hook. Changes: - Koha::Schema::Util::ExceptionTranslator → Koha::Schema::ExceptionMapper - Update Koha::Schema to use the new module - Move test file accordingly - Remove now-empty Koha/Schema/Util/ directory Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #17 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198624 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198624&action=edit Bug 42391: (QA follow-up): Remove unused Try::Tiny from Koha::Item Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #18 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198625 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198625&action=edit Bug 42391: (QA follow-up): Clarify exception_action ref-passthrough comment Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #19 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 198626 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198626&action=edit Bug 42391: (QA follow-up): Fix unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |pedro.amorim@openfifth.co.u | |k --- Comment #20 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Please fix the last 2 commits author information. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198620|0 |1 is obsolete| | Attachment #198621|0 |1 is obsolete| | Attachment #198622|0 |1 is obsolete| | Attachment #198623|0 |1 is obsolete| | Attachment #198624|0 |1 is obsolete| | Attachment #198625|0 |1 is obsolete| | Attachment #198626|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #21 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201763&action=edit Bug 42391: Use exception_action to translate DBIC exceptions at the source DBIx::Class provides an exception_action hook that intercepts every throw_exception call before it becomes a DBIx::Class::Exception. By installing this hook on Koha::Schema via a connection() override, all database exceptions are automatically translated into Koha exceptions at the source. This eliminates the need for translate_exception catch blocks in every caller: - Remove try/catch from Koha::Object::store - Remove try/catch from Koha::Object::delete - Remove try/catch from Koha::Item::add_to_bundle - Remove translate_exception convenience method from Koha::Schema - Remove safe_do (already removed in previous follow-up) ExceptionTranslator is refactored to work on plain error message strings instead of DBIx::Class::Exception objects, since exception_action receives the message before wrapping. Unmatched errors fall through to DBIx::Class::Exception->throw() as the default, preserving backward compatibility. The warn $_->{msg} that was in Object::store is no longer needed since exceptions are translated before they propagate. Tests that asserted on that warning (via warning_like) are updated to use throws_ok directly. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Object.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Item.t => SUCCESS: Tests pass! k$ prove t/db_dependent/Koha/Patron/Modifications.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #22 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201764 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201764&action=edit Bug 42391: (follow-up) Fix Objects.t case 3 to test FK deletion properly The 'Delete a patron that cannot be deleted' test (case 3) was using a stale $r_us object from case 2 (not in storage), so it was not actually testing FK constraint deletion. The assertion on DBIx::Class::Exception passed by accident. This follow-up: - Fetches a fresh Koha::Patron object for case 3 - Clears $e_us/$e_them before the new test - Asserts Koha::Exceptions::Object::FKConstraintDeletion, which is what exception_action now produces for FK deletion errors Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #23 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201765&action=edit Bug 42391: (follow-up) Add NotNull, NotInStorage and UnhandledDBError translations Extend the ExceptionTranslator to handle two additional MySQL error patterns: - NOT NULL violations ('Column X cannot be null' and 'X doesn't have a default value') → Koha::Exceptions::Object::NotNull - Not in database errors → Koha::Exceptions::Object::NotInStorage Add Koha::Exceptions::Object::NotNull exception class with a property field identifying the offending column. Add Koha::Exceptions::Object::UnhandledDBError as a catch-all for DBI errors that don't match any known pattern. This closes the gap where unmatched MySQL errors could leak as DBIx::Class::Exception to callers (e.g. templates that no longer catch that type). The exception_action handler now distinguishes three cases: - Koha exception objects (from txn_do re-throws): passed through via 'die $msg' to preserve the original exception type. Without this, txn_do's rollback-and-rethrow would stringify the Koha exception and wrap it in UnhandledDBError, losing the specific type. - DBI errors (prefixed 'DBI Exception:'): translated if possible, otherwise wrapped in UnhandledDBError. - DBIC internal errors (e.g. 'Not in database'): translated if possible, otherwise thrown as DBIx::Class::Exception. Note: NotInStorage translation works for update operations but not for delete-on-detached-Row where DBIC bypasses the Schema's exception_action. Those cases remain DBIx::Class::Exception. Update tests: - OAIServers.t, Z3950Servers.t: assert NotNull with property check - Hold.t: assert NotNull, remove warn suppression hack - Objects.t: update-not-in-storage cases assert NotInStorage - Upload.t: delete-not-in-storage stays DBIx::Class::Exception Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #24 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201766&action=edit Bug 42391: (follow-up) Rename ExceptionTranslator to ExceptionMapper The Koha::Schema::Util:: namespace adds unnecessary nesting for a single-purpose class. This follow-up moves it to Koha::Schema::ExceptionMapper, which sits alongside Koha::Schema itself — a better fit given it's tightly coupled to the Schema's exception_action hook. Changes: - Koha::Schema::Util::ExceptionTranslator → Koha::Schema::ExceptionMapper - Update Koha::Schema to use the new module - Move test file accordingly - Remove now-empty Koha/Schema/Util/ directory Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #25 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201767 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201767&action=edit Bug 42391: (QA follow-up): Remove unused Try::Tiny from Koha::Item Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #26 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201768 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201768&action=edit Bug 42391: (QA follow-up): Clarify exception_action ref-passthrough comment Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #27 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201769 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201769&action=edit Bug 42391: (QA follow-up): Fix unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42391 --- Comment #28 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Pedro Amorim (ammopt) from comment #20)
Please fix the last 2 commits author information.
Done, it was not obvious! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org