[Bug 11032] New: Validate passed records in Biblio.pm
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Bug ID: 11032 Summary: Validate passed records in Biblio.pm Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: MARC Bibliographic data support Assignee: gmcharlt@gmail.com Reporter: colin.campbell@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org We've all seen Biblio.pm giving the error 'can't call method fields on an undefined value' for various reasons. They usually cause confusion or panic as a low level error gets thrown at the user with no apparent relation to their current operation. The problem is that many of Biblio's routines are rather optimistically written so that it thinks you've passed a MARC::Record object you have and it assumes that nothing could every go wrong in this best of all possible worlds. Routines should at least check they have a defined record before trying to process it, and should log a stack trace so that the cause can be determined in the case of the error. Merely passing a low level error to the end user is non-productive. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|gmcharlt@gmail.com |colin.campbell@ptfs-europe. | |com --- Comment #1 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 21958 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21958&action=edit Proposed Patch As these cases do not need to access db have added a Biblio.t which test error returns and logs messages to stderr, to run prove t/Biblio.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 --- Comment #2 from Owen Leonard <oleonard@myacpl.org> --- I'm happy to see that you're trying to address this problem. Can you please add a test plan to your commit message? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 --- Comment #3 from Colin Campbell <colin.campbell@ptfs-europe.com> --- (In reply to Owen Leonard from comment #2)
I'm happy to see that you're trying to address this problem. Can you please add a test plan to your commit message?
test plan is the tests added to t/Biblio.t see previous comment -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21958|0 |1 is obsolete| | --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 22447 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22447&action=edit [SIGNED OFF] Bug 11032: Check a valid MARC::Record passed to Biblio Intermittently problems in the calling environment cause a C4::Biblio to be called with and undefined MARC::Record object. This results in the process dieing and returning to the enduser a low level message such as 'cannot call method x on an undefined object' For exported subroutines taking a MARC::Record object check that object is defined otherwise return a logical return value and log a stack trace to the error log. A couple of cases were checking but dieing, this may have unwelcome results in a persistent environment so croak has been downgraded to carp Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Adds lots of checks for $record in various places, should not affect behaviour. Passed all tests and QA script, including new unit tests. Tested adding and saving a new record. Also tested detail and result pages without XSLT. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22447|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 22832 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22832&action=edit Bug 11032: Check a valid MARC::Record passed to Biblio Intermittently problems in the calling environment cause a C4::Biblio to be called with and undefined MARC::Record object. This results in the process dieing and returning to the enduser a low level message such as 'cannot call method x on an undefined object' For exported subroutines taking a MARC::Record object check that object is defined otherwise return a logical return value and log a stack trace to the error log. A couple of cases were checking but dieing, this may have unwelcome results in a persistent environment so croak has been downgraded to carp Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Adds lots of checks for $record in various places, should not affect behaviour. Passed all tests and QA script, including new unit tests. Tested adding and saving a new record. Also tested detail and result pages without XSLT. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #6 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Colin! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11032 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #7 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Patch pushed to 3.14.x, will be in 3.14.2. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org