[Bug 29857] New: We must stringify our exceptions correctly
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Bug ID: 29857 Summary: We must stringify our exceptions correctly Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: In Discussion Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 21022 We must have a generic full_message method in the Koha::Exceptions base class to display all the info we have about the exception. The current state is bad as we are displaying the "description" of the exception but it does not contain useful information. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21022 [Bug 21022] Exceptions should skip stringifying if message manually passed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129329 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129329&action=edit Bug 29857: Make the exception classes inherit from the base class We have a base class Koha::Exceptions::Exception but it's not used in most of our exception classes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129330&action=edit Bug 29857: Provide a generic full_message method for our exceptions We pass useful information when throwing exceptions, but we don't display them! We must display all the details we need when the exception is stringified. Test plan: Use the following snippet: use Koha::Exceptions::Elasticsearch; Koha::Exceptions::Elasticsearch::BadResponse->throw( type => "a good type", details => "and some details" ); Execute it without and with the patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- [Bad response received when submitting request to Elasticsearch] vs Exception 'Koha::Exceptions::Elasticsearch::BadResponse' thrown 'Bad response received when submitting request to Elasticsearch' with type => a good type, details => and some details -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This seems a solid improvement to me.. honestly I've found our exceptions confusing to work with.. creating new ones and all the classes all over the place.. this works towards making them all consistent at least. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- What are the next steps here, discussion wise. I'm on board with the change myself and would happily sign off as is.. were there any particular points you wanted to raise/question Jonathan? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Ready for signoff then :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129329|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=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129330|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=29857 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130007 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130007&action=edit Bug 29857: Make the exception classes inherit from the base class We have a base class Koha::Exceptions::Exception but it's not used in most of our exception classes Signed-off-by: Tomas 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=29857 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130008 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130008&action=edit Bug 29857: Provide a generic full_message method for our exceptions We pass useful information when throwing exceptions, but we don't display them! We must display all the details we need when the exception is stringified. Test plan: Use the following snippet: use Koha::Exceptions::Elasticsearch; Koha::Exceptions::Elasticsearch::BadResponse->throw( type => "a good type", details => "and some details" ); Execute it without and with the patch. Signed-off-by: Tomas 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=29857 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130009&action=edit Bug 29857: Rename base class as Koha::Exception Signed-off-by: Tomas 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=29857 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130010 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130010&action=edit Bug 29857: Unit tests Signed-off-by: Tomas 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=29857 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130011 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130011&action=edit Bug 29857: Fix behavior This patch makes: - passed parameters be rendered alphabetically to allow testing. - only the defined parameters be rendered in the string - the method honor the fact that $self->message should be returned if passed to the constructor (i.e. if the developer called Koha::Exceptions::MyException->throw( $string ) they will expect $string to be rendered. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas 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=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130010|0 |1 is obsolete| | --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130012 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130012&action=edit Bug 29857: Unit tests Signed-off-by: Tomas 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=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130011|0 |1 is obsolete| | --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130013 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130013&action=edit Bug 29857: Fix behavior This patch makes: - passed parameters be rendered alphabetically to allow testing. - only the defined parameters be rendered in the string - the method honor the fact that $self->message should be returned if passed to the constructor (i.e. if the developer called Koha::Exceptions::MyException->throw( $string ) they will expect $string to be rendered. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas 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=29857 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I renamed the 'base' exception class to Koha::Exception as it is something I wanted to do for a long time already. Beware the package/file name doesn't need to match the exception name in Exception::Class, so we could've left Koha::Exceptions::Exception and still use Koha::Exception->throw if we named it like that. But... this is certainly better :-D I tried to highlight the expected behaviors with the tests and found some glitches, that are fixed on the 'Fix behavior' patch. Hope you agree with the proposed follow-ups. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130012|0 |1 is obsolete| | --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130014 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130014&action=edit Bug 29857: Unit tests Signed-off-by: Tomas 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=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130013|0 |1 is obsolete| | --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130015 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130015&action=edit Bug 29857: Fix behavior This patch makes: - passed parameters be rendered alphabetically to allow testing. - only the defined parameters be rendered in the string - the method honor the fact that $self->message should be returned if passed to the constructor (i.e. if the developer called Koha::Exceptions::MyException->throw( $string ) they will expect $string to be rendered. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas 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=29857 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Dude, this is a great set of follow-ups. Will work through them properly and test it tomorrow -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30007 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30007 [Bug 30007] Make ->anonymize methods throw an exception if AnonymousPatron is not set -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |BLOCKED QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #0)
We must have a generic full_message method in the Koha::Exceptions base class to display all the info we have about the exception. The current state is bad as we are displaying the "description" of the exception but it does not contain useful information.
Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with status => x, type => HASH(0x5653eb753278) How useful is HASH(0x5653eb753278) ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looks quite good to me. I am wondering if we should remove the confusion between singular and plural completely now. We have a base class Koha::Exception. And we could define subclasses in Koha/Exception/ like Koha/Exception/Generic.pm to contain those now in Exceptions.pm. And the rest already in Exceptions/. What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 130156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130156&action=edit Bug 29857: (CONCEPT) Switch to singular, add a load method The load method calls the Class::Exception import. Allowing you to skip a use Class::Exception everywhere, and define 'nicer' exception lists? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #21)
Created attachment 130156 [details] [review] Bug 29857: (CONCEPT) Switch to singular, add a load method
The load method calls the Class::Exception import. Allowing you to skip a use Class::Exception everywhere, and define 'nicer' exception lists?
Any thoughts ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 130157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130157&action=edit Bug 29857: (QA follow-up) Add license -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130156|0 |1 is obsolete| | --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 130158 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130158&action=edit Bug 29857: (CONCEPT) Switch to singular, add a load method The load method calls the Class::Exception import. Allowing you to skip a use Class::Exception everywhere, and define 'nicer' exception lists? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #25 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- IMO we are going too far here. Please provide patches on follow-up bug reports instead. I don't understand why the "Virtualshelves exceptions" part is in Koha::Exception::Generic. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #26 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #19)
(In reply to Jonathan Druart from comment #0)
We must have a generic full_message method in the Koha::Exceptions base class to display all the info we have about the exception. The current state is bad as we are displaying the "description" of the exception but it does not contain useful information.
Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with status => x, type => HASH(0x5653eb753278)
How useful is HASH(0x5653eb753278) ?
We should call Dumper if it's a struct. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #27 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #26)
(In reply to Marcel de Rooy from comment #19)
(In reply to Jonathan Druart from comment #0)
We must have a generic full_message method in the Koha::Exceptions base class to display all the info we have about the exception. The current state is bad as we are displaying the "description" of the exception but it does not contain useful information.
Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with status => x, type => HASH(0x5653eb753278)
How useful is HASH(0x5653eb753278) ?
We should call Dumper if it's a struct.
Marcel, did you find an example in the code or it's theoretical? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #27)
(In reply to Jonathan Druart from comment #26)
(In reply to Marcel de Rooy from comment #19)
(In reply to Jonathan Druart from comment #0)
We must have a generic full_message method in the Koha::Exceptions base class to display all the info we have about the exception. The current state is bad as we are displaying the "description" of the exception but it does not contain useful information.
Exception 'Koha::Exception::TestMR' thrown 'Something went wrong!' with status => x, type => HASH(0x5653eb753278)
How useful is HASH(0x5653eb753278) ?
We should call Dumper if it's a struct.
Marcel, did you find an example in the code or it's theoretical?
I do not have a real example. So I am not sure about that. But it isn't far fetched if we are starting to use this more. Dumper could do it indeed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #25)
IMO we are going too far here. Please provide patches on follow-up bug reports instead.
I don't understand why the "Virtualshelves exceptions" part is in Koha::Exception::Generic.
Because it is now in Exceptions.pm where it does not belong either :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #25)
IMO we are going too far here. Please provide patches on follow-up bug reports instead.
Heh. I see this happen all the time :) As to singular, I think we do half of the work here and then leave it. And we should try more to finish what we started? But you can remove the concept patch. Not sure if it will reach another report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30057 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30057 [Bug 30057] Move Virtualshwlves exceptions to their own file -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130157|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=29857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130158|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=29857 --- Comment #31 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130355 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130355&action=edit Bug 29857: (QA follow-up) Add license Signed-off-by: Tomas 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=29857 --- Comment #32 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #22)
(In reply to Marcel de Rooy from comment #21)
Created attachment 130156 [details] [review] [review] Bug 29857: (CONCEPT) Switch to singular, add a load method
The load method calls the Class::Exception import. Allowing you to skip a use Class::Exception everywhere, and define 'nicer' exception lists?
Any thoughts ?
I like it. It deserves its own bug IMHO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #33 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #25)
I don't understand why the "Virtualshelves exceptions" part is in Koha::Exception::Generic.
I filed bug 30057 and submitted patches. Test plz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130007|0 |1 is obsolete| | --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131112 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131112&action=edit Bug 29857: Make the exception classes inherit from the base class We have a base class Koha::Exceptions::Exception but it's not used in most of our exception classes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130008|0 |1 is obsolete| | --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131113 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131113&action=edit Bug 29857: Provide a generic full_message method for our exceptions We pass useful information when throwing exceptions, but we don't display them! We must display all the details we need when the exception is stringified. Test plan: Use the following snippet: use Koha::Exceptions::Elasticsearch; Koha::Exceptions::Elasticsearch::BadResponse->throw( type => "a good type", details => "and some details" ); Execute it without and with the patch. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130009|0 |1 is obsolete| | --- Comment #36 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131114 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131114&action=edit Bug 29857: Rename base class as Koha::Exception Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130014|0 |1 is obsolete| | --- Comment #37 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131115 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131115&action=edit Bug 29857: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130015|0 |1 is obsolete| | --- Comment #38 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131116 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131116&action=edit Bug 29857: Fix behavior This patch makes: - passed parameters be rendered alphabetically to allow testing. - only the defined parameters be rendered in the string - the method honor the fact that $self->message should be returned if passed to the constructor (i.e. if the developer called Koha::Exceptions::MyException->throw( $string ) they will expect $string to be rendered. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/Koha/Exceptions.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130355|0 |1 is obsolete| | --- Comment #39 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131117 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131117&action=edit Bug 29857: (QA follow-up) Add license Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|m.de.rooy@rijksmuseum.nl |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #40 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Glad to see the next stage put onto it's own bug... The code here works well and clear things up a lot. QA scripts are happy, tests passing. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #41 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 131232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131232&action=edit Bug 29857: (QA follow-up) Fix unit test Object.t Unit test failling with : # Failed test 'Exception message correct' # at /kohadevbox/koha/t/db_dependent/Koha/Object.t line 335. # got: 'Exception 'Koha::Exception' thrown 'Asked to embed items but its return value doesn't implement to_api' # ' # expected: 'Asked to embed items but its return value doesn't implement to_api' # Looks like you failed 1 test of 29. Fixed by using $@->message like in other places. prove t/db_dependent/Koha/Object.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #42 from Fridolin Somers <fridolin.somers@biblibre.com> --- There are other fails in test suite but I push master, we will fix them -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #43 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #44 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131240&action=edit Bug 29857: (follow-up) Fix Barcode_transform_hooks.t Signed-off-by: Tomas 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=29857 --- Comment #45 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131241&action=edit Bug 29857: (follow-up) Fix t/Koha/Plugins/Tab.t Signed-off-by: Tomas 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=29857 --- Comment #46 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131242 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131242&action=edit Bug 29857: (follow-up) Fix t/db_dependent/Koha/Biblio.t Signed-off-by: Tomas 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=29857 --- Comment #47 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131243 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131243&action=edit Bug 29857: (follow-up) Fix t/Koha/Object/Message.t Signed-off-by: Tomas 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=29857 --- Comment #48 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131244 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131244&action=edit Bug 29857: (follow-up) Fix t/Koha/Email.t Signed-off-by: Tomas 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=29857 --- Comment #49 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131256 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131256&action=edit Bug 29857: (follow-up) Fix t/db_dependent/Auth.t Signed-off-by: Tomas 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=29857 --- Comment #50 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131258 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131258&action=edit Bug 29857: (follow-up) Fix t/db_dependent/Koha/Patron/Modifications.t Signed-off-by: Tomas 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=29857 --- Comment #51 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Just in case: I'm submitting the fixes one by one just in case I need to leave and cannot complete the fixes. But this could just be pushed on a single commit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #52 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #51)
Just in case: I'm submitting the fixes one by one just in case I need to leave and cannot complete the fixes. But this could just be pushed on a single commit.
These one line patches read so beautifully ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #53 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 131287 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131287&action=edit Bug 29857: (follow-up) Fix t/Koha/REST/Plugin/Query.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #54 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 131288 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131288&action=edit Bug 29857: (follow-up) Fix t/db_dependent/Koha/Plugins/Plugins.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #55 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 131289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131289&action=edit Bug 29857: (follow-up) Fix t/db_dependent/Koha/Club/Hold.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #56 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 131290 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131290&action=edit Bug 29857: (follow-up) Fix t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #57 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 131291 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131291&action=edit Bug 29857: (follow-up) Fix t/db_dependent/api/v1/transfer_limits.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 --- Comment #58 from Fridolin Somers <fridolin.somers@biblibre.com> --- Follow-ups pushed to master :
git log --oneline --no-decorate --grep=29857 be74dd2edf Bug 29857: (follow-up) Fix t/db_dependent/api/v1/transfer_limits.t 94220261f2 Bug 29857: (follow-up) Fix t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t b5560af25e Bug 29857: (follow-up) Fix t/db_dependent/Koha/Club/Hold.t 6ccf04661a Bug 29857: (follow-up) Fix t/db_dependent/Koha/Plugins/Plugins.t e93de8268e Bug 29857: (follow-up) Fix t/Koha/REST/Plugin/Query.t 13a49f860f Bug 29857: (follow-up) Fix t/db_dependent/Koha/Patron/Modifications.t d406051dcb Bug 29857: (follow-up) Fix t/db_dependent/Auth.t 8d2c00fb02 Bug 29857: (follow-up) Fix t/Koha/Email.t e568e8ef5d Bug 29857: (follow-up) Fix t/Koha/Object/Message.t 42c592f3ad Bug 29857: (follow-up) Fix t/db_dependent/Koha/Biblio.t bcd98589c1 Bug 29857: (follow-up) Fix t/Koha/Plugins/Tab.t 651a2228aa Bug 29857: (follow-up) Fix Barcode_transform_hooks.t c287121471 Bug 29857: (QA follow-up) Fix unit test Object.t 6ece7185b0 Bug 29857: (QA follow-up) Add license 3951145cf7 Bug 29857: Fix behavior c10640e8a7 Bug 29857: Unit tests 2b9fae5445 Bug 29857: Rename base class as Koha::Exception 7429f28cfc Bug 29857: Provide a generic full_message method for our exceptions 42096deea4 Bug 29857: Make the exception classes inherit from the base class
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29857 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org