QA Comment:
Fridolin, although our coding guidelines do not say anything about the use of eval, I do not think that this is the most elegant solution. It also will degrade performance btw.
The actual problem is somewhere else and I would prefer to solve it there.

In this case the corrupted record will be discovered by GetMarcBiblio (it also contains an eval already btw!).
Note that GetMarcBiblio returns undef now, but this return value is NOT checked by UpdateTotalIssues. So here we come closer.. 
UpdateTotalIssues should test that value and not just say $record->field..
(There will be [much] more instances where this happens.]

So, we could do at least two things now:
[1] Return from UpdateTotalIssues: return if !$record; the cronjob continues. (Note that there is a warn in GetMarcBiblio.)
[2] Also return a value to the caller (note that only the cronjob and Circulation.pm call this routine). Print the number of skipped records in the cronjob.

Setting status to Failed QA

(In reply to M. de Rooy from comment #3)
Thanks for your advice, I agree 100%.

Here we are, new patches.
I've added a simple unit test on C4::Biblio::UpdateTotalIssues.

One may create more complex unit tests to cover all cases.

I have the impression that the improved patch became obsoleted and do not know if it was qa-ed ?

Created attachment 32695 [details] [review]
[PASSED QA] Bug 12995 - script update_totalissues.pl stops on corrupted record

When running update_totalissues.pl cronjob, it will stop on a corrupted
record.

This patch changes UpdateTotalIssues so that it return 1 if processing
record has succeded. Also, if mapping with biblioitems.totalissues does
not exist, the method has nothing to do so it stops and returns 1.

When processing a corrupted record, script now alerts about the error on
this biblionumber (if script is verbose) and process next record.
A total number of records with error will be printed at the end of the
script.

Test plan :
- Create a dabase with a few biblios and some issues
- Modify first biblio record (use direct sql update) : set empty value
  in biblioitems.marcxml
- Launch script : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v

=> Without patch : the script stops at first record
=> With patch : the script prints error for first record and processes
   all records

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
I was able to confirm the problem before the patch and successfully
follow the test plan.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 32696 [details] [review]
[PASSED QA] Bug 12995 - script update_totalissues.pl stops on corrupted record - UT

This patch adds an unit test on C4::Biblio::UpdateTotalIssues method
call on non existing record

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
I can't comment on the correctness of the test other than to say it ran
successfully.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Improves CLI script, works as described.

Note: A small improvement would be to output the problematic biblionumber.

Sorry, my fault - missed to attach the first patch.

Patches pushed to master.

Thanks Fridolin!

(In reply to Katrin Fischer from comment #14)
> Created attachment 32696 [details] [review] [review]
> [PASSED QA] Bug 12995 - script update_totalissues.pl stops on corrupted
> record - UT
> 
> This patch adds an unit test on C4::Biblio::UpdateTotalIssues method
> call on non existing record
> 
> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
> I can't comment on the correctness of the test other than to say it ran
> successfully.
> 
> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
> Improves CLI script, works as described.
> 
> Note: A small improvement would be to output the problematic biblionumber.

This patch causes the package builds to fail, as it requires a database.

(In reply to Robin Sheat from comment #17)
> This patch causes the package builds to fail, as it requires a database.
Oh, carp.
How can we fix the UT ? mock the db ?

(In reply to Fridolin SOMERS from comment #18)
> (In reply to Robin Sheat from comment #17)
> > This patch causes the package builds to fail, as it requires a database.
> Oh, carp.
> How can we fix the UT ? mock the db ?

See bug 13230. There's a patch for this.