1280682 - TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js and test-retest-config.js and test-instrument-setup.js

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Jorg K (CEST = GMT+2)]

First seen here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=d6147076fb29cb4162ea9343658f19e5004f6508

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js | test-mail-account-setup-wizard.js::test_mail_account_setup

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js | test-mail-account-setup-wizard.js::test_bad_password_uses_old_settings

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-retest-config.js | test-retest-config.js::test_re_test_config

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\instrumentation\test-instrument-setup.js | test-instrument-setup.js::test_mail_account_setup

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\instrumentation\test-instrument-setup.js | test-instrument-setup.js::teardownModule

Although these failures have associated bugs, there seems to be a systemic problem that these all started going perma-orange at the same time.

mozmake SOLO_TEST=account/test-mail-account-setup-wizard.js mozmill-one
mozmake SOLO_TEST=account/test-retest-config.js mozmill-one
mozmake SOLO_TEST=instrumentation/test-instrument-setup.js mozmill-one
fail locally. They all hang when looking up the e-mail provider.

Regression windows M-C:
Last good: moz:14c5bf11d37b
First bad: moz:b9f4f3806395

Comment 1

[Jorg K (CEST = GMT+2)]

In a local build I tried to set up an account for [e-mail] and it fails to find the details. Usually Gmail is well known.

Comment 2

[aleth [:aleth]]

(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #0)
> Although these failures have associated bugs, there seems to be a systemic
> problem that these all started going perma-orange at the same time.

Yeah, this doesn't look like the existing intermittent bugs.

Comment 3

[aleth [:aleth]]

Looks like at least some of these (I didn't check them all) fail locally too. (make SOLO_TEST=... mozmill-one)

Comment 4

[Jorg K (CEST = GMT+2)]

(In reply to aleth [:aleth] from comment #3)
> Looks like at least some of these fail locally
All fail locally as per comment #0. Also note comment #1 that the account creation function seems to be broken which is reflected in the test failures.

Comment 5

[Jorg K (CEST = GMT+2)]

OK, when I try to create a local account as per comment #1 I get this on the debug console:

-- Exception object --
+ _message (string) ''
+ stack (string) 465 chars
+ code (number) 0
+ uri (string) 'http://autoconfig.gmail.com/mail/config-v1.1.xml'
+ constructor (function) 6 lines
+ message (string) ''
+ toString (function) 4 lines
*
-- Stack Trace --
Exception@chrome://messenger/content/accountcreation/util.js:137:5
ServerException@chrome://messenger/content/accountcreation/fetchhttp.js:262:3
FetchHTTP.prototype._response@chrome://messenger/content/accountcreation/fetchhttp.js:190:19
FetchHTTP.prototype.start/request.onerror@chrome://messenger/content/accountcreation/fetchhttp.js:117:36
MsgAccountManager@chrome://messenger/content/accountUtils.js:255:9
oncommand@chrome://messenger/content/messenger.xul:1:1
Bad response content
-- Exception object --
+ _message (string) 'Bad response content'
+ stack (string) 464 chars
+ code (number) -4
+ uri (string) 'https://live.mozillamessaging.com/autoconfig/v1.1/gmail.com'
+ constructor (function) 6 lines
+ message (string) 'Bad response content'
+ toString (function) 4 lines
*
-- Stack Trace --
Exception@chrome://messenger/content/accountcreation/util.js:137:5
ServerException@chrome://messenger/content/accountcreation/fetchhttp.js:262:3
FetchHTTP.prototype._response@chrome://messenger/content/accountcreation/fetchhttp.js:190:19
FetchHTTP.prototype.start/request.onload@chrome://messenger/content/accountcreation/fetchhttp.js:116:35
MsgAccountManager@chrome://messenger/content/accountUtils.js:255:9
oncommand@chrome://messenger/content/messenger.xul:1:1
Bad response content
-- Exception object --
+ _message (string) 'Bad response content'
+ stack (string) 464 chars
+ code (number) -4
+ uri (string) 'https://live.mozillamessaging.com/autoconfig/v1.1/google.com'
+ constructor (function) 6 lines
+ message (string) 'Bad response content'
+ toString (function) 4 lines
*
-- Stack Trace --
Exception@chrome://messenger/content/accountcreation/util.js:137:5
ServerException@chrome://messenger/content/accountcreation/fetchhttp.js:262:3
FetchHTTP.prototype._response@chrome://messenger/content/accountcreation/fetchhttp.js:190:19
FetchHTTP.prototype.start/request.onload@chrome://messenger/content/accountcreation/fetchhttp.js:116:35
MsgAccountManager@chrome://messenger/content/accountUtils.js:255:9
oncommand@chrome://messenger/content/messenger.xul:1:1

Comment 6

[Jorg K (CEST = GMT+2)]

Alice, can you locate the regression?
It seems that account creation in TB can't lookup the ISP database any more so some tests fail and it also doesn't work manually.

https://treeherder.mozilla.org/#/jobs?repo=comm-central
Last good: Wed Jun 15, 14:02:26 - [e-mail]
First bad: Wed Jun 15, 22:12:29 - [e-mail]
There you need at the nightlies that failed the tests. Clicking on the job details, you get the revisions of M-C.

Regression windows M-C:
Last good: moz:14c5bf11d37b
First bad: moz:b9f4f3806395

I've been looking at the pushlog but I can't see anything obvious.

Thanks in advance for your help.

Comment 7

[Alice0775 White]

Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=14c5bf11d37b9e92d27f7089d9392de2ac339bb3&tochange=b9f4f38063951cd5a8b249911aea61869f40fd1f
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=abd1ef9761da&tochange=d6147076fb29

Via local build
Last Good : c-c d6147076fb29, m-c 4b35974b5b39
First Bad : c-c d6147076fb29, m-c 87aee9d5e04d

Regressed by: 87aee9d5e04d	Thomas Wisniewski — Bug 918733 - have overrideMimeType throw INVALID_STATE_ERR if the XHR is in the DONE or LOADING states. r=sicking

Comment 8

[Jorg K (CEST = GMT+2)]

Thomas and Jonas: Your change in bug 918733 broke the Thunderbird account manager. I don't know whether you're familiar with Thunderbird. We have an account manager, which, after the user enters their e-mail address, accesses the configuration database at https://live.mozillamessaging.com/autoconfig/v1.1/ to look up details about the servers for this e-mail address. For Gmail it would for example retrieve smtp.gmail.com as outgoing server. You can see error messages in comment #5.

Our code is in
https://dxr.mozilla.org/comm-central/source/mailnews/base/prefs/content/accountcreation/fetchhttp.js

Can you please advise us which changes we need to make to get our software working again or at least how to debug it.

I read in bug 918733 comment #1 related to an earlier patch:
  Note that Chrome now throws whether also specifying a charset or not.

Does that mean that it is meant not to work at all any more? Since Thunderbird can be viewed as one big Firefox add-on, would your change not also affect other add-ons?

Comment 9

[Jorg K (CEST = GMT+2)]

We call overrideMimeType() here:
https://dxr.mozilla.org/comm-central/source/mailnews/base/prefs/content/accountcreation/fetchhttp.js#141

Comment 10

[Thomas Wisniewski]

Your call to overrideMimeType isn't actually doing anything, since it's too late for it to have any impact once the document is loading/done. As MDN states, you have to call overrideMimeType before send(). It was a no-op before, now we're just throwing an exception rather than silently failing (which is what the WhatWG specification requires).

Unless a lot of unmaintained addons start breaking, I'd advise keeping the change. It's good to inform users when they're using overrideMimeType incorrectly, whether they're writing addons or not (especially since other browsers will throw an exception here).

Comment 11

[Jorg K (CEST = GMT+2)]

I commented out our
  this._request.overrideMimeType("text/xml");
and that does change the behaviour (it hangs completely now) which is surprising since you said that it was a no-op before.

Comment 12

[Jorg K (CEST = GMT+2)]

Note that before, this was handled differently:
https://hg.mozilla.org/comm-central/diff/078dd301fa0c/mailnews/base/prefs/content/accountcreation/fetchhttp.js#l140

Comment 13

[Jorg K (CEST = GMT+2)]

Aceman, can you take a look at this, https://wiki.mozilla.org/Thunderbird/Community_Members lists you for "account manager (UI)". This is old Bienvenu code, and the two lines in question here are by Joshua, see comment #12.

Comment 14

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #11)
> I commented out our
>   this._request.overrideMimeType("text/xml");
> and that does change the behaviour (it hangs completely now) which is
> surprising since you said that it was a no-op before.
Oh, ignore that, I must have mucked something up in the build. Taking the line out makes it work.

Comment 15

[Jorg K (CEST = GMT+2)]

Attachment: This seems to work.

No idea who has expertise in this area. Apparently overrideMimeType() was a no-op, so no damage done removing it ;-) With this change, account creation works again.

Comment 16

[Jorg K (CEST = GMT+2)]

Thomas, thanks for your help!

Comment 17

[aleth [:aleth]]

Comment on attachment 8763305 [details] [diff] [review]
This seems to work.

Review of attachment 8763305 [details] [diff] [review]:
-----------------------------------------------------------------

I don't know this code, but I suspect this has revealed a bug that needs fixing (seeing bugs are referenced in the change history).

I'm OK with removing the noop to fix the test though, as that just restores the status quo.

::: mailnews/base/prefs/content/accountcreation/fetchhttp.js
@@ +134,5 @@
>          if (!mimetype)
>            mimetype = "";
>          mimetype = mimetype.split(";")[0];
>          if (mimetype == "text/xml" ||
> +            mimetype == "application/xml")

What happened to text/rdf?

Comment 18

[Jorg K (CEST = GMT+2)]

(In reply to aleth [:aleth] from comment #17)
> What happened to text/rdf?
I thought rdf isn't xml, so not a good idea to pass it to JXON.build().

Comment 19

[Magnus Melin [:mkmelin]]

Comment on attachment 8763305 [details] [diff] [review]
This seems to work.

Review of attachment 8763305 [details] [diff] [review]:
-----------------------------------------------------------------

I'd leave text/rdf

RDF has multiple serializations, one of which is XML. text/rdf without +subtype is basically wrong but as this is dealing with wrongly configured servers it's probably best to just leave it in.

Comment 20

[Jorg K (CEST = GMT+2)]

Attachment: This seems to work (v2).

Comment 21

[Jorg K (CEST = GMT+2)]

Aleth, please land at you leisure. No need to rush since today's nightly is already on the way.

Comment 22

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/cb066e3a2889c0eb93fc1c84748615b065a9ee6c
Bug 1280682 - Remove call to overrideMimeType() due to bug 918733. r=mkmelin

Comment 23

[Thomas Wisniewski]

(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #16)
> Thomas, thanks for your help!

No problem, sorry the change crept up on you like that. I'm working on other XHR-related spec stuff, so I'll try to keep you guys in mind if other patches of mine might break this code for some reason.