Last Comment Bug 571800 - undo delete gives IMAP error: "Error in IMAP command UID: Invalid UID messageset"
: undo delete gives IMAP error: "Error in IMAP command UID: Invalid UID message...
Status: RESOLVED FIXED
: regression
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: unspecified
: x86 Windows XP
: -- normal (vote)
: Thunderbird 3.3a1
Assigned To: David :Bienvenu
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-13 16:16 PDT by Dan Halbert
Modified: 2012-06-20 03:06 PDT (History)
7 users (show)
mozilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
.1+
.1-fixed
unaffected


Attachments
imap.log showing the error (search for "Error") (35.44 KB, text/plain)
2010-06-13 16:18 PDT, Dan Halbert
no flags Details
proposed fix (1.02 KB, patch)
2010-06-14 18:08 PDT, David :Bienvenu
standard8: review+
standard8: superreview+
standard8: approval‑thunderbird3.1.1+
Details | Diff | Splinter Review
fix to server and test case to catch this error (3.42 KB, patch)
2010-06-17 17:24 PDT, David :Bienvenu
standard8: review+
Details | Diff | Splinter Review

Description Dan Halbert 2010-06-13 16:16:57 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.70 Safari/533.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100608 Thunderbird/3.1

Undo after a delete reports an error from the IMAP UID command:

Error in IMAP command UID: Invalid UID messageset.

Reproducible: Always

Steps to Reproduce:
. Select a message in INBOX or other folder (except Trash)
2. Delete the message.
3. Do Undo (^Z)
4. A notification window rolls up, saying (for example):

"The current operation on 'Inbox' did not succeed. The mail server for account testing@halwitz.org responded: Error in IMAP command UID: Invalid UID messageset."



My IMAP server is secure.emailsrvr.com, which is Rackspace's commercial Noteworthy IMAP service.

I could give you a test account on that service if you'd like; contact me.
Comment 1 Dan Halbert 2010-06-13 16:18:20 PDT
Created attachment 450960 [details]
imap.log showing the error (search for "Error")
Comment 2 Dan Halbert 2010-06-13 16:21:16 PDT
This error happens in 3.1RC2; operating on the same mailbox in 3.0.4 does not cause the problem.
Comment 3 Dan Halbert 2010-06-13 16:23:59 PDT
This is reminiscent of bug 569588 (same initial scenario), but is not a crash.
Comment 4 Ludovic Hirlimann [:Usul] 2010-06-14 04:04:05 PDT
Dan can you find out when this regressed ?
Comment 5 Dan Halbert 2010-06-14 06:17:58 PDT
(In reply to comment #4)
> Dan can you find out when this regressed ?

I tried rc1, and then bisected. I ended up testing all the releases:
3.04   OK
3.1a1  OK
3.1b1  OK
3.1b2  error as reported
3.1rc1 error as reported
3.1rc2 error as reported

** ALSO, I accidentally confirmed the crash in bug 569588 while testing. As that bug says, the following scenario causes a crash:

1. Delete message
2. QUICKLY do undo (^Z)

In this scenario, I did not see the IMAP UID error notification. However, I was able to reliably reproduce the bug 569588 crash all the way back to Thunderbird 3.0.2, so it is not clear how/if it is related to this IMAP error bug. The IMAP error bug happens when the undo is not done quite so quickly (perhaps there is time for a round-trip to the IMAP server).
Comment 6 Ludovic Hirlimann [:Usul] 2010-06-14 07:10:54 PDT
Dan can you try to finegrain the bisect using nightlies so we end up with a shorter regression window ?
Comment 7 Dan Halbert 2010-06-14 07:44:26 PDT
(In reply to comment #6)
> Dan can you try to finegrain the bisect using nightlies so we end up with a
> shorter regression window ?

Sure. I assume you mean the nightlies here:
  http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/
but which branch should I be testing? There's -comm-1.92, comm-1.9.2-i10n, etc. Thanks.

I will be able to do this some time this evening Eastern time.
Comment 8 Ludovic Hirlimann [:Usul] 2010-06-14 07:47:07 PDT
Yes -comm-1.9.2 is the one to use.
Comment 9 David :Bienvenu 2010-06-14 15:49:06 PDT
I was able to reproduce this - we're using the message id and not the uid in the command to clear the deleted flag. So someone (probably me) got confused about what a string contained...
Comment 10 Dan Halbert 2010-06-14 15:51:01 PDT
(In reply to comment #9)
> I was able to reproduce this ...

Would bisecting still be helpful? I was just about to start doing that.
Comment 11 David :Bienvenu 2010-06-14 15:55:50 PDT
(In reply to comment #10)

> Would bisecting still be helpful? I was just about to start doing that.

It might be helpful, but it might not be needed either. I'll spend a few minutes investigating and let you know if I get stumped.
Comment 12 David :Bienvenu 2010-06-14 16:09:25 PDT
I think it was the patch in bug 525646 that caused the regression. bisection won't be helpful at this point, but thanks for offering.
Comment 13 David :Bienvenu 2010-06-14 18:08:23 PDT
Created attachment 451184 [details] [diff] [review]
proposed fix

this fixes it - running the unit tests to make sure it doesn't regress anything. But they passed w/ the bug in :-)
Comment 14 David :Bienvenu 2010-06-14 21:14:33 PDT
I think what's happening here is that the regression was from a long time ago, but we never saw a ui manifestation of it because the undo runs w/o a message window. Thus, the error from the server was not displayed. But now that we have a non-modal alert mechanism, even urls run without a msgWindow cause the error to get displayed to the user. It's unfortunate that this is in 3.1, but it's probably not worth respinning for.

Some imap servers may not generate an alert in this case, since they might just ignore bad UID's in the STORE command.
Comment 15 Wayne Mery (:wsmwk, NI for questions) 2010-06-15 02:59:42 PDT
Might the "response" be different depending on server?  With undo I get 

 The current operation on <folder> did not succeed. The server for account <name> responded: Bogus sequence in UID FETCH: UID sequence syntax error.
Comment 16 Wayne Mery (:wsmwk, NI for questions) 2010-06-15 04:22:50 PDT
forgot to mention...in 3.1 (not sure about 3.2) this causes this UI to freeze for a few seconds
Comment 17 David :Bienvenu 2010-06-17 14:56:10 PDT
I need to figure out why our unit test didn't catch this, other than the fact that the undo actually works, despite the error.
Comment 18 David :Bienvenu 2010-06-17 15:02:38 PDT
fakeserver ignores invalid uids, which it should not, I don't think, so I'll have to patch fakeserver.
Comment 19 David :Bienvenu 2010-06-17 17:24:17 PDT
Created attachment 452145 [details] [diff] [review]
fix to server and test case to catch this error
Comment 20 David :Bienvenu 2010-06-17 17:25:19 PDT
marking blocking 3.1.1
Comment 21 Mark Banner (:standard8) 2010-06-21 03:08:07 PDT
(In reply to comment #14)
> But now that we have
> a non-modal alert mechanism, even urls run without a msgWindow cause the error
> to get displayed to the user.

You're right, I couldn't understand it at first, but yes, I did miss that when we swapped to non-modal.
Comment 22 David :Bienvenu 2010-06-22 06:54:45 PDT
fixed on trunk
Comment 23 WADA 2010-06-22 10:22:26 PDT
Adding text in bug summary, for ease of tracking and search.
Comment 24 Mark Banner (:standard8) 2010-06-22 10:29:12 PDT
(In reply to comment #23)
> Adding text in bug summary, for ease of tracking and search.

Sorry but I really don't think we need to know which specific versions in the summary, especially when branch statuses are governed by existing fields.
Comment 25 Mark Banner (:standard8) 2010-07-12 04:22:51 PDT
David: is this ready for 3.1 branch?
Comment 26 David :Bienvenu 2010-07-12 14:41:25 PDT
Comment on attachment 451184 [details] [diff] [review]
proposed fix

yes, I think this is ready for 3.1.1
Comment 27 Mark Banner (:standard8) 2010-07-13 01:53:15 PDT
Checked into comm-1.9.2 and relbranch for 3.1.1 build 2:

http://hg.mozilla.org/releases/comm-1.9.2/rev/143c3b884937
http://hg.mozilla.org/releases/comm-1.9.2/rev/8f2aea2d5c4b

Note You need to log in before you can comment on or make changes to this bug.