833708 - [OTA] No error shown when no space left on device to download the update

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[rafael.marquez]

STR:
1-Try to fill the system memory until only 20MB are left
2-Tap to download a new update when it´s available

Expected result --> Receive some kind of error message stating that no space left on device

Actual result --> Update download "starts" as usual. You can see the following message in the |adb logcat|

I/Gecko   (   96): DirectoryProvider: Error: No volume found with 109293980 bytes for downloading update Firefox 18.0a2
I/Gecko   (   96): UpdatePrompt: Error downloading update Firefox 18.0a2: 0
I/Gecko   (   96): UpdatePrompt: Update error, state: null, errorCode: 0
I/Gecko   (   96): UpdatePrompt: Setting gecko.updateStatus: null
I/GeckoDump(   96): XXX FIXME : Got a mozContentEvent: update-available-result

Comment 1

[Jason Smith [:jsmith]]

This might be a dupe. There's a general bug I think on the fact that we don't do a good job reporting errors when an update fails. The error fails to show up at the right time, and instead randomly shows up in later usage. Something like "There was an error downloading your updates."

Comment 2

[Julien Wajsberg [:julienw]]

CCing Dave as well, we recently talked about error handling, he might have a good idea.

Comment 3

[Julien Wajsberg [:julienw]]

Normally, when we get an error while downloading (the "update-error" mozChromeEvent), we should have a SystemBanner message appearing. Maybe the reporter didn't see it because it disappear after a few seconds.

At least the phone is still usable and not left in an unusable state. But it won't have system updates any more and the user doesn't know this if he missed the banner. The banner isn't even specific to system updates, the same banner is displayed for app updates.

Comment 4

[Marshall Culpepper [:marshall_law]]

Attachment: gecko errorCode fix + test - v1

first stab at fixing the behavior of the swallowed FILE_TOO_BIG error by setting
the active update's errorCode directly. the new test verifies the behavior, but
I also need to verify this for gaia.

Comment 5

[Dave Hylands [:dhylands]]

Comment on attachment 716264 [details] [diff] [review]
gecko errorCode fix + test - v1

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

::: b2g/components/DirectoryProvider.js
@@ +103,5 @@
>    },
>  
>    findUpdateDirWithFreeSpace: function dp_findUpdateDirWithFreeSpace(requiredSpace, subdir) {
>      if (!Services.volumeService) {
> +      log("Warning: Volume service is not running");

This would be normal on B2G-desktop

::: b2g/components/UpdatePrompt.js
@@ +314,5 @@
>      }
>  
>      log("Error downloading update " + aUpdate.name + ": " + aUpdate.errorCode);
> +    let errorCode = aUpdate.errorCode >>> 0;
> +    if (errorCode == FILE_ERROR_TOO_BIG) {

Shouldn't this be Cr.NS_ERROR_FILE_TOO_BIG ?

::: toolkit/mozapps/update/test/unit/xpcshell.ini
@@ +31,5 @@
>  [test_bug595059.js]
>  skip-if = toolkit == "gonk"
>  reason = custom nsIUpdatePrompt
>  [test_bug794211.js]
> +[test_bug833708.js]

This should be a gonk-only test right?

Comment 6

[Marshall Culpepper [:marshall_law]]

Attachment: gaia update error fix - v1

the 'update-error' event is being sent back to gaia on the same stack as gaia's
call to 'update-download-result', which makes the gaia call to 
removeFromDownloadsQueue() happen before addToDownloadsQueue(). updated unit
tests to reflect this.

Comment 7

[Marshall Culpepper [:marshall_law]]

Attachment: gecko errorCode fix + test - v2

this has been tested and confirmed working with gaia now (see new gaia patch)

Comment 8

[Dietrich Ayala (:dietrich)]

Re-nominating, blocks bug 838247, which is a blocker.

Comment 9

[Marshall Culpepper [:marshall_law]]

Giving this a try run because of the new xpcshell test:
 https://tbpl.mozilla.org/?tree=Try&rev=df032249d7e9

Comment 10

[Marshall Culpepper [:marshall_law]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/feed6a5160f7

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/feed6a5160f7

Comment 12

[John Ford [:jhford] CET/CEST Berlin Time]

Did the Gaia portion of this patch land on master?

Comment 13

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/ec6d010f907a69059853b20c36a5117cf890e8d5

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/729a6f7ebb5d
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/f0c54cd4dd70

Leaving the status flags as affected for the gaia bits that need uplift still.

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifting commit ec6d010f907a69059853b20c36a5117cf890e8d5:
v1-train: 8e55fe64809d73b9dbd78344c4324cee366530c7
v1.0.1: fac1a3ad598e49081d3273c271021290f2574390

Comment 16

[:CSchmoeckel]

Adding flag moztrap+ because there is an adequate test case in MozTrap for this bug. 

https://moztrap.mozilla.org/manage/cases/?filter-id=4184

Comment 17

[amiller]

Verified fixed on 
Unagi Build ID: 20130403070201
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/512258bc00a3
Gaia: daea430624ec02f8d36a12d581fc4a3278c27cb7


and

Unagi Build ID: 20130403070204
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/d467369d1b0c
Gaia: 06e0e5ce42bdfb62bdbe38271de6b5b2d9e40e75

Error shown when no space left on device to download the update. Does not Repro.