Bug 21530 - overdue_notices.pl aborting because error is not caught
Summary: overdue_notices.pl aborting because error is not caught
Status: RESOLVED WORKSFORME
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Colin Campbell
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-10 10:42 UTC by Colin Campbell
Modified: 2024-07-09 13:09 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Patch to test a valid object returned (1.11 KB, patch)
2018-10-10 10:55 UTC, Colin Campbell
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2018-10-10 10:42:58 UTC
Saw this triggered by a situation where a branch had been removed but some rules remained. However when doing a lookup the return should always be tested for validity. Because it wasnt when methods were called on null the overdues_notices process aborts with a runtime error and subsequent branches overdues are not processed.
Untested returns are one of the biggest causes of errors and often hard to track down errors in the system .
Comment 1 Colin Campbell 2018-10-10 10:55:55 UTC
Created attachment 80326 [details] [review]
Patch to test a valid object returned

Adds a run-time test to ensure the assumption of the following code is true.
Without the patch process aborts on call to branchemail method.

With patch applied if find fails to return a valid library we skip to the next and issue an error listing the problematic branchcode

As we dont live in the "best of all possible worlds" code should be more pessimistic and test for unexpected results
Comment 2 Hayley Pelham 2019-03-08 02:28:31 UTC
(In reply to Colin Campbell from comment #1)
> Created attachment 80326 [details] [review] [review]
> Patch to test a valid object returned
> 
> Adds a run-time test to ensure the assumption of the following code is true.
> Without the patch process aborts on call to branchemail method.
> 
> With patch applied if find fails to return a valid library we skip to the
> next and issue an error listing the problematic branchcode
> 
> As we dont live in the "best of all possible worlds" code should be more
> pessimistic and test for unexpected results

Hi, is there some test plan for this patch?
Comment 3 Liz Rea 2019-04-30 00:47:59 UTC
Hi Colin,

I'm wondering if this situation will have been helped by the fix for bug 22679 - thoughts?
Comment 4 Owen Leonard 2019-07-31 16:27:40 UTC
I'm marking this Failed QA because comment 2 and comment 3 need to be addressed before this can be signed off.
Comment 5 Martin Renvoize (ashimema) 2024-07-09 13:09:14 UTC
Pretty sure this is caught at this point.. either way I'm closing and we can open a fresh bug when we spot it in the wild.