503193 - Bad recipient info in outbox blocks further sending with mailnews.sendinBackground

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Joe Sabash [:JoeS1]]

Noticed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090706 Lightning/1.0pre Shredder/3.0b3pre ID:20090706031725

I have been using mailnews.sendinBackground for some time with no problems.
Here are the circumstances that led to an illegal recipient in the outbox:

After replying to a newsgroup post on mozilla.dev.apps.thunderbird I later noticed the post had a follow-up to mozilla.dev.apps.calendar
Most likely, I used the sent copy, edited as new and tried to delete the calendar part of the address, and type in thunderbird.
This somehow resulted in a message in the outbox with just thunderbird as the recipient, which of course failed to send.
The above steps are not clear, and probably another bug.

However in this state, no further sending to any account is possible.
Setting mailnews.sendinBackground to false freed everything up.

I saved the outbox, but it has cued up private addresses that I don't want public. Should be possible to reproduce by manually editing the outbox and setting proper flags.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

This might want to block, given that

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

this is an ugly failure mode for a feature that we want to emphasize that will effectively appear to Tb2 users as a regression.  It will depend on how this rates in the list of bugs that we end up triaging, I expect.

Comment 3

[Mark Banner (:standard8)]

I could have sworn there was a bug on failing to send one message would not send others, but I can't find it at the moment. I was planning on fixing that anyway.

The issue I'm more concerned about was he newsgroup post - Joe, did you get an alert to say that the message send failed?

Comment 4

[Joe Sabash [:JoeS1]]

(In reply to comment #3)

> The issue I'm more concerned about was he newsgroup post - Joe, did you get an
> alert to say that the message send failed?

Yes. 2 in fact. 
"A news(NNTP) error occurred: Posting failed."
Then:
"Send message error Connecting failed. Message could not be posted..yada, yada"

The thing is, that's on the bad message, and there are more valid messages after
that sitting in the outbox.

I did get one "silent failure" just after I flipped the pref to true again.
But after a re-start the alerts came up again OK

Comment 5

[David Ascher (:davida)]

blocks a blocking bug.

Comment 6

[Mark Banner (:standard8)]

We've decided that send in background isn't going to be switched on by default for the 3.0 release. Therefore taking off the blocking list - it'll still get fixed for send in background before that lands as it blocks the implementation bug.

Comment 7

[Wayne Mery (:wsmwk)]

standard8 in  comment #3
> I could have sworn there was a bug on failing to send one message would not
> send others, but I can't find it at the moment. I was planning on fixing that
> anyway.

probably  Bug 151364 -  Sending unsent messages doesn't try to send other mails if one fails

Comment 8

[David :Bienvenu]

Odd, the code looks like it tries to advance to the next message even in the error case. I'll try to reproduce it.

Comment 9

[David :Bienvenu]

Attachment: keep sending queued messages even on error

This allows us to keep going even when one message fails to send. It leaves the message that failed to send in the outbox, so the next time you try to send unsent messages, it will try again. In this user's case, he'd need to delete the bad message from the unsent messages folder, but I prefer that to us automatically deleting messages we couldn't send.

In the case of no network connection or the server being down, now we will try to keep sending messages. I don't know how big a deal that is. With automatic network state detection, it should be rare. Perhaps we should ask if the user wants to keep trying to send messages. We could try to be smart about the error we get, though that can be problematic. But this patch as is removes one more impediment to turning on send in background.

Comment 10

[David :Bienvenu]

http://hg.mozilla.org/comm-central/rev/d0a085ce5b78

Comment 11

[David :Bienvenu]

Attachment: fix unit test failures

So I know you wanted to get rid of the status, but I'm reluctant to drop error statuses since I'd much rather send them and have the listener have the option of ignoring them.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 557684 [details] [diff] [review]
fix unit test failures

I think this is fine as well.

Comment 13

[Mark Banner (:standard8)]

Checked in the additional patch: http://hg.mozilla.org/comm-central/rev/2ac6f82f3906

Comment 14

[[e-mail]]

Error status should ALL be non-modal, that is the operations should continue in the background, sending all valid recipient messages while flagging the invalid one for attention/repair.

Comment 15

[Mark Banner (:standard8)]

(In reply to [e-mail] from comment #14)
> Error status should ALL be non-modal, that is the operations should continue
> in the background, sending all valid recipient messages while flagging the
> invalid one for attention/repair.

That's a separate issue to this one. Please check for previously filed bugs, and if not, file a new bug.

Comment 16

[Jonathan Kamens]

See bug 749654.