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

RESOLVED FIXED in Bugzilla 2.18

Status

()

task
P1
blocker
RESOLVED FIXED
16 years ago
7 years ago

People

(Reporter: hauser, Assigned: justdave)

Tracking

unspecified
Bugzilla 2.18
Bug Flags:
approval +

Details

Attachments

(1 attachment)

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
perl "Software Error" messages are by definition critical.  Not confirming yet
because I haven't attempted to reproduce yet.
Severity: normal → critical
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
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?
Yes,if the user doesn't exist.

What is $person in the case where it dies?
bumping priority... this is causing production problems on very high visilibity
bugs on b.m.o
Severity: critical → blocker
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.18
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 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.
Attachment #147205 - Flags: review?(myk)
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)
Attachment #147205 - Flags: review?(myk) → review+
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.13; previous revision: 1.12
done
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: approval+
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.