227172 - Can't call method "id" on an undefined value at Bugzilla/BugMail.pm line 708. in sanitycheck.cgi?rescanallBugMail=1

Status: RESOLVED FIXED

(Bugzilla :: Administration, task, P1)

Description

[Ralf Hauser]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031129
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031129

sanity check ran ok and found a few bugs with potentially unsent mail.

So, I click on the link below that info to "Send These Emails".

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Actual Results:  
Bugzilla Sanity Check
	  	
OK, now attempting to send unsent mail

16 bugs found with possibly unsent mail.

Software error:

Can't call method "id" on an undefined value at Bugzilla/BugMail.pm line 708.

For help, please send mail to the webmaster

Expected Results:  
no error

just upgraded from 2.16.3

Comment 1

[Dave Miller [:justdave]]

perl "Software Error" messages are by definition critical.  Not confirming yet
because I haven't attempted to reproduce yet.

Comment 2

[Max Kanat-Alexander]

I was unable to reproduce this on the 2.17.6 cvs-tip today.

I made three bugs, all reported by and assigned to me.

I ran this SQL in the database manually, to simulate that all bugs had unsent email:

UPDATE bugs SET delta_ts = '20040119182832', lastdiffed = '2003-01-01';

Then I ran the sanity check, and clicked on "send these messages" (or whatever
that link is called). It ran fine.

I would recommend WORKSFORME, unless you have additional data, Ralf?

-M

Comment 3

[Matthew Tuck [:CodeMachine]]

Best culprit I can find is this:

    my $user = Bugzilla::User->new_from_login($person);
    my $userid = $user->id;

Can new_from_login return undef?

Comment 4

[Bradley Baetz (:bbaetz)]

Yes,if the user doesn't exist.

What is $person in the case where it dies?

Comment 5

[Dave Miller [:justdave]]

bumping priority... this is causing production problems on very high visilibity
bugs on b.m.o

Comment 6

[Dave Miller [:justdave]]

OK, here's the scenario we nailed down for what was causing it on b.m.o:

It basically comes down to a race condition between a user changing their email
address between the time a bug change is submitted and when the email gets sent
for it.

In the case we ran into on b.m.o, something had gone wrong in the original
sending of the email (in this case, Apache timed out and killed it because there
were tons of CCs), and in the time between then and when the cron job kicked in
to send unsent mail, a user who had been removed from the CC changed their email
address.

Since they were removed from the CC, it pulled their address from the activity
log (which shows the address at the time it was removed) to send them mail to
let them know they were removed, and then tried to get the userid for that
address, which of course no longer existed.

We solved it on b.m.o by patching it to silently drop the email address in
question (just skip it if the account doesn't exist).

Comment 7

[Dave Miller [:justdave]]

Attachment: Patch v1

Comment 8

[Dave Miller [:justdave]]

Comment on attachment 147205 [details] [diff] [review]
Patch v1

This implements what's described in comment 6, and is what's currently applied
to b.m.o.

Comment 9

[Jouni Heikniemi]

Comment on attachment 147205 [details] [diff] [review]
Patch v1

>     my $user = Bugzilla::User->new_from_login($person);
>+    if (!$user) { # person doesn't exist, probably changed email
>+      return;
>+    }
>     my $userid = $user->id;
> 
>     $seen{$person} = 1;

Offhand, I'd say it's better to return after $seen{$person} is set, so that the
inexistence of a user gets cached. That's a very marginal issue however, so
either way is fine for me. 

r=jouni (by inspection, I cannot easily test this)

Comment 10

[Dave Miller [:justdave]]

Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.13; previous revision: 1.12
done