Closed Bug 1280682 Opened 4 years ago Closed 4 years ago

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

Categories

(Thunderbird :: Account Manager, defect)

defect
Not set

Tracking

(thunderbird49 unaffected, thunderbird50 affected)

RESOLVED FIXED
Thunderbird 50.0
Tracking Status
thunderbird49 --- unaffected
thunderbird50 --- affected

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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
In a local build I tried to set up an account for huhu@gmail.com and it fails to find the details. Usually Gmail is well known.
(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.
Looks like at least some of these (I didn't check them all) fail locally too. (make SOLO_TEST=... mozmill-one)
(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.
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
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 - rsx11m.pub@gmail.com
First bad: Wed Jun 15, 22:12:29 - stefanh@inbox.com
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.
Flags: needinfo?(alice0775)
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
Blocks: 918733
Flags: needinfo?(alice0775)
Component: General → Account Manager
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?
Flags: needinfo?(wisniewskit)
Flags: needinfo?(jonas)
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).
Flags: needinfo?(wisniewskit)
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.
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.
Flags: needinfo?(jonas) → needinfo?(acelists)
(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.
Attached patch This seems to work. (obsolete) — Splinter Review
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.
Flags: needinfo?(acelists)
Attachment #8763305 - Flags: review?(mkmelin+mozilla)
Attachment #8763305 - Flags: review?(acelists)
Attachment #8763305 - Flags: feedback?(aleth)
Thomas, thanks for your help!
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?
Attachment #8763305 - Flags: feedback?(aleth)
(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 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.
Attachment #8763305 - Flags: review?(mkmelin+mozilla)
Attachment #8763305 - Flags: review?(acelists)
Attachment #8763305 - Flags: review+
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #8763314 - Flags: review+
Attachment #8763305 - Attachment is obsolete: true
Aleth, please land at you leisure. No need to rush since today's nightly is already on the way.
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
(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.
You need to log in before you can comment on or make changes to this bug.