Oddly formatted dependency emails

RESOLVED FIXED in Bugzilla 2.18

Status

()

P3
blocker
RESOLVED FIXED
16 years ago
6 years ago

People

(Reporter: bugreport, Assigned: wicked)

Tracking

2.17.1
Bugzilla 2.18
All
Other
Bug Flags:
approval +
blocking2.18 +

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
Received the following notification. 



Date: Sun, 10 Nov 2002 05:56:33 -0800 (PST)
Message-Id: <200211101356.gAADuXn10512@mothra.mozilla.org>
From: bugzilla-daemon@mozilla.org
To: bugreport@peshkin.net
Subject: [Bug 98304] Allow Bugzilla to work with PostgreSQL (PgSQL)
X-Bugzilla-Reason: CC

http://bugzilla.mozilla.org/show_bug.cgi?id=98304


patl@cag.lcs.mit.edu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Bug 98304 depends on bug 114696, which changed state.

Bug 114696 Summary: Permission checking in queries not optimal
http://bugzilla.mozilla.org/show_bug.cgi?id=114696

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXEDBug 98304 depends on bug
114696, which changed state.

Bug 114696 Summary: Permission checking in queries not optimal
http://bugzilla.mozilla.org/show_bug.cgi?id=114696

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
(Reporter)

Updated

16 years ago
Blocks: 179176
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.18
This may be another buffer issue :-)

Gerv

Comment 2

16 years ago
I tried to reproduce this, but didn't really have much luck (I didn't try that
long either, though, so...)

Gerv: are you saying you think this is a buffering issue in processmail or in
templating or... ?

joel and I were talking about it on IRC and were semi-confused.

One other thing interesting tidbit from that conversation:

<joel> actually, it is stranger than that...
<joel> I think this happened when a completely unrelated bug changed state
<joel> the person who is said to have made the change did nothing to wither of
theose 2 bugs

Hrm...
(Reporter)

Comment 3

16 years ago
This isn't just oddly formatted, it seems to be spillover from an unrelated event.

Adding patl@cag.lcs.mit.edu to the bug and asking the question....

We are trying to figure out what was going on at the time this email was
generated.  Were you doing anything potentially related at the time?
Severity: major → blocker
Priority: P2 → P1
(Reporter)

Comment 4

16 years ago

1 more note on the triggering event,

patl was added to the cc list of bug 98304 at... 2002-11-10 05:56:29

(Reporter)

Comment 5

16 years ago

Bug 71794 looks like a really good candidate for triggerring this.

Comment 6

16 years ago
All I did was add myself to the CC list of bug 98304.  Or at least, that is what
I *tried* to do.

On the first attempt, it failed because I had inadvertently changed the
"milestone" field to 2.20.  So I clicked "back" in my browser, set the milestone
back to 2.18, and typed my Email address again, and hit Enter.

That worked, but the bizarre notification got sent...
(Reporter)

Comment 7

16 years ago
Patrick:

  Yep, that makes sense.  Was the add to the CC list done by viewing the bug and
adding youself or was it using the buglist "change serveral at once" to add
yourself?

(I'm wondering if the trasition-checking in process_bug was treating
"dontchange" as differrent than the old status)

Comment 8

16 years ago
I viewed the bug and added myself.
Yeah, I've had this a few times, always on dependency-changed mail. I can't see
what causes this, though - processmail runs as a separate process, and we've now
reenabled buffering everywhere (processmail sets $|=1, too), so there shouldn't
be anything which stays arround as persisted.

Even if we were sending mails twice, they'd be separate mails, I would have thought
Actually.....

I submitted that particular bug (bug 114696) as FIXED just before bmo shutdown,
and that went through, but the dependency emails didn't get sent - they were
caught by the shutdownhtml check (and showed as sent to noone/__UNKNOWN__)

All the bugs I've got this for have been dependent on that bug, or dpeendent on
a dependant of that bug; I'm wondering if thats what caused this, combined with
some breakage when processmail ends up triggereing the dependency thing.

Can people get this on testbed installs, by playing with delta_ts + lastchanged?

patl@cag.lcs.mit.edu, was there no indication in that mail about your cc change?
The other hting ot thikn about is that processmail does:

           $thisdiff =
              "\nBug $id depends on bug $depbug, which changed state.\n\n" .

Note the leading \n, which appears to have somehow vanished in the resulting mail??

Comment 12

16 years ago
Created attachment 105773 [details]
Sample bogus mail

Bogus Email message I received when I added myself to the CC list of the bug.

Comment 13

16 years ago
Hm.  Looking at the headers, and come to think of it, I also voted for the bug...

So I voted for the bug, and then I went back and added myself to the CC list. 
The message in the attachment was the only Email notification I received, however.
Is bug 71794 still under suspicion here?

Gerv
gerv: Not sure, but I think its unlikely

from myk's post-shutdown bmo db, we have:

mysql> SELECT bug_id, delta_ts, lastdiffed FROM bugs WHERE bug_id IN (98304,
114696);
+--------+----------------+---------------------+
| bug_id | delta_ts       | lastdiffed          |
+--------+----------------+---------------------+
|  98304 | 20021107203846 | 2002-11-08 18:03:18 |
| 114696 | 20021108180933 | 2002-11-08 18:09:38 |
+--------+----------------+---------------------+

Not sure why 98304's lastdiffed was updated at 18:03, but its clear that bug
98304 didn't manage to send out the dep mail. 
(Reporter)

Comment 16

16 years ago
WRT comment 14, I'll answer in the style of a crummy cop movie...
There's no evidence that it is the culprit, but it shouln't leave town :-)

(Reporter)

Comment 17

16 years ago
Downgrading priority and severity due to the following...

1) All the people notified by the strange emails would have reasonably had
access to at least some of these bugs.

2) Both know cases where this has occurred involve a bug that was being modified
just prior to the shutdown and major upgrade.  It probably would pay to run
processmail --rescanall immediately prior to upgrade.

We seem to be converging on observations that a bug had a bugmail pending just
as the system was shut down and something odd happened after that.

If correct, this would be a rather isolate incident.


Severity: blocker → major
Priority: P1 → P3
Until someone proves otherwise, this isn't a regression fromthe recent upgrade
No longer blocks: 179176
Just a datapoint, but I've gotten three or four more messages with these same
formatting errors since the upgrade.

Has anyone else seen these since the upgrade?
JP: have all those mails been related to bug 114696 or its deps?
No, actually.

I'll attach a couple of mails that showed this behavior, but that don't have
anything to do with that bug (AFAICT).
Created attachment 106096 [details]
Another oddly formatted email
Hmm. That was changing dep + resolution in one mail. Did that confuse something?
You know... I was thinking about this... this isn't just some weird special case
of bug 133368, is it?
I don't know. It may be related, but we're getting one mail with the _same_ data
twice, not two mails.

Has anyone reproded this when changing dep + resolution in one go?
*** Bug 190059 has been marked as a duplicate of this bug. ***

Comment 27

16 years ago
Dup bug 190059 has more exact information on what might be causing this.

I've just marked bug 189563 as blocking 59652 at the same time marking it as
RESOLVED FIXED (it was in "ASSIGNED" state). The bugmail it generated for bug
59652 (see below) is completely messed up:
1) It does *not* say that a new bug was added to its dependency list
2) The "state change" message is repeated twice
3) The "state change" message includes NEW -> ASSIGNED transition that happened
well before the dependency was added (so it shouldn't be included).

The bugmail I received was the following:
------
http://bugzilla.mozilla.org/show_bug.cgi?id=59652


mozilla-bugs@nogin.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Bug 59652 depends on bug 189563, which changed state.

Bug 189563 Summary: use of uninitialized value found by valgrind
http://bugzilla.mozilla.org/show_bug.cgi?id=189563

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXEDBug 59652 depends on bug
189563, which changed state.

Bug 189563 Summary: use of uninitialized value found by valgrind
http://bugzilla.mozilla.org/show_bug.cgi?id=189563

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
Summary: Oddly formatted emails from BMO → Oddly formatted emails from BMO (when marking as blocking and resolving in a single commit ?)
Bumping severity back to blocker.  This needs to be fixed before we can call
Bugzilla version 2.18.

I just got mail like this again (will attach momentarily)

Bug 84876 was set as depending on bug 124174 a long time ago.  All that was done
in the resolving action was to resolve the bug.  Bug 84876 just generated one of
these emails when bug 124174 was resolved.  This eliminates setting as a dep and
resolving at the same time as the specific cause (adjusting summary accordingly).
Severity: major → blocker
Summary: Oddly formatted emails from BMO (when marking as blocking and resolving in a single commit ?) → Oddly formatted dependency emails from BMO
Created attachment 114075 [details]
another weird dep mail

notice in here at the top it says:

"guenter-huerkamp@web.de changed:"

When in fact it was preed@sigkill.com who made the change....

this is nasty when it's putting the wrong person in here, too.
I just got a mail almost identical to this from Bugscape, which is running the
CVS tip as of Sunday night, so it's not limited to b.m.o, and it's not fixed on
the tip, even with the new BugMail.pm code.
Summary: Oddly formatted dependency emails from BMO → Oddly formatted dependency emails
We need steps to repro this before we can fix it.

Comment 32

16 years ago
Do steps in comment #27 (add another bug in "bug xxx blocks" box, select
RESOLVED FIXED and hit "Commit" and see what you get as a bugmail for that
"another" bug) work?
Just a note to make sure we get this down for posterity: there were some musings
on IRC that this may be caused by/related to bug 192877.
Status: NEW → ASSIGNED
*** Bug 220339 has been marked as a duplicate of this bug. ***

Comment 35

15 years ago
Let me copy from the duped bug report I produced:

So here is my suggestion how to solve it. If we want to combine different
incidents in one mail (I don't think we should, but this is not the question
here), then each incident should be given completly, seperated by a clear marker
like a line like this:

--------------------- change n ------------------------------------

pi
re comment 35:

I don't think anyone is considering combining bug email reports; the emails you
got are is a symptom of a bug, not a "feature."
*** Bug 133368 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 38

15 years ago
AFAICS, the problem is in Bugzilla/Bugmail.pm around line 273 where it generates
the dependencies change diff data and around line 227 where it generates normal
field change diff data. Oddly formatted email is generated when there is both
normal field change (BugsThisDependsOn) and dependency change.

Problem happens because the dependency differ uses only text hash key to create
diffpart and header hash value will contain the same data the normal field
differ used. Now the diffparts will contain two entries and both of them have
same header. Email generation at around line 767 will filter out the other
header because it's same so that counts for the one extra What/Removed/Added header.

I still don't know why the duplication of dependency data happens but I suspect
it's because of the same problem. I'll try to find out more and produce a patch
that fixes the problem and probably clarifies what I'm trying to say here..
Unless I'm totally wrong on this, of course. :)
(Assignee)

Comment 39

15 years ago
Created attachment 144244 [details] [diff] [review]
Fix

I think I was right.. This patch fixes the odd formatting of emails by clearing
the variable used for diffing. Now the dependency email will also show fields
that were changed (usually only BugsThisDependsOn field).
Assignee: preed → wicked
Hmmm.... I think you might be on to something. :)

Would it achieve the same end to localize $diffpart within the foreach loop at
line 229?  Since it's getting pushed onto @diffparts before leaving that loop,
the $diffpart generated within it doesn't seem to have any use beyond that
point, and that seems to be where the leftover is coming from...

Created attachment 144314 [details] [diff] [review]
Fix v2

That was indeed it. :)

I revised it just slightly...  there were only two places that $diffpart was
being used, so it's now declared locally within the scope of each of the two
places using it, which is logically a little cleaner than just clearing it
before the second use. :)

Tested on landfill: can reproduce 100% by making any given bug block bug 1 at
the same time as resolving it (the bugmail for bug 1 would then be the "funky"
one) before applying this patch.  With this patch applied, it looks nice and
pretty :)
Attachment #144244 - Attachment is obsolete: true
Flags: blocking2.18+
Attachment #144314 - Flags: review?(bugreport)
(Reporter)

Comment 42

15 years ago
Comment on attachment 144314 [details] [diff] [review]
Fix v2

Good job!
r=joel
Attachment #144314 - Flags: review?(bugreport) → review+

Updated

15 years ago
Flags: approval?
Flags: approval? → approval+

Comment 43

15 years ago
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.11; previous revision: 1.10
done
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
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.