1623398 - DNS.jsm should not return -1 on errors

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, task)

Description

[Magnus Melin [:mkmelin]]

Found during debugging of account setup, DNS.jsm can return -1 and we don't handle that properly. It's not very nice to return mixed types.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1623398_DNS_minusone.patch

Comment 2

[Paul Morris [:pmorris]]

Comment on attachment 9134192 [details] [diff] [review]
bug1623398_DNS_minusone.patch

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

Nice catch, changes look fine.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3ac3b884565b
DNS.jsm should not return -1 on errors, just an empty array. r=pmorris

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fa2ff756eb23
followup - remove now irrelevant test. rs=bustage-fix DONTBUILD

Comment 5

[Patrick Cloke [:clokep]]

I believe this is trying to differentiate between a valid response with no records and an invalid / error response. Are we sure we want to treat these the same?

Where do we not handle this properly? Is the bug in the handler, not in the DNS code?

Comment 6

[Magnus Melin [:mkmelin]]

That's what it was used for, but I do think wrongly so. If it was a case we want to handle it should have thrown an Error instead of returning a different type than expected.
For the one case we were prepared for the -1, the only difference it made was a log statement and for other parts ended up doing the same as an empty array. The case where we didn't handle it properly I found in the account config code: https://searchfox.org/comm-central/rev/6ead4697de9615629cc9065d161126ba2d1ea057/mail/components/accountcreation/content/fetchConfig.js#246,248. That could of course be handled differently if we wanted, but it seems like a needless complication.

Comment 7

[Patrick Cloke [:clokep]]

Alright. Thanks for the explanation!

Comment 8

[Magnus Melin [:mkmelin]]

Oh, and many seemingly normal calls seem to return -1,

var {DNS} = ChromeUtils.import("resource:///modules/DNS.jsm");
DNS.mx("localhost").then(recs => {
alert(JSON.stringify(recs));
});

DNS.srv("thunderbird.net").then(recs => {
alert(JSON.stringify(recs));
});