Closed Bug 570173 Opened 11 years ago Closed 11 years ago

Errors from findUpdates aren't passed out to the listener correctly

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla2.0b1
Tracking Status
blocking2.0 --- final+

People

(Reporter: mossop, Assigned: mossop)

References

Details

(Whiteboard: [AddonsRewrite])

Attachments

(1 file, 2 obsolete files)

The error value should be passed in onUpdateFinished and we should still send all the other events in the case of an error.
Attached patch patch rev 1 (obsolete) — Splinter Review
Found a couple of things while writing the docs and this test. We weren't throwing the sensible error on http errors and when there was an error onNoCompatibilityUpdate was not getting sent. We also have the error code being passed on the wrong event in some cases.

I've also fixed bug 553869 here. It makes sense that the error codes from findUpdates should be defined on AddonManager even if they do match those on AddonUpdateChecker as that is what we use internally.
Attachment #449300 - Flags: review?(robert.bugzilla)
Blocks: 553869
Status: NEW → ASSIGNED
Attached patch patch rev 2 (obsolete) — Splinter Review
This one actually passes all tests. test_bug384052.js was a little broken, it should never find an update and should verify that there was an error (since the url it tries to use doesn't exist as we don't start the test server).
Attachment #449300 - Attachment is obsolete: true
Attachment #449320 - Flags: review?(robert.bugzilla)
Attachment #449300 - Flags: review?(robert.bugzilla)
Comment on attachment 449320 [details] [diff] [review]
patch rev 2

Bah, still not quite right.
Attachment #449320 - Attachment is obsolete: true
Attachment #449320 - Flags: review?(robert.bugzilla)
Attached patch patch rev 3Splinter Review
Ok final one. Forgot that we can't label the checkCert failure as a security error since then it wouldn't match all the other ssl related errors so might as well just lump them all as download errors.
Attachment #449349 - Flags: review?(robert.bugzilla)
Attachment #449349 - Flags: review?(robert.bugzilla) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/9ec599d9a1f0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Target Milestone: mozilla1.9.3a5 → mozilla1.9.3
Marking as verified fixed based on check-in and a green tinderbox in our area.
Status: RESOLVED → VERIFIED
Target Milestone: mozilla1.9.3 → mozilla1.9.3a6
You need to log in before you can comment on or make changes to this bug.