When updating an app, if it fails (network goes down) sometimes the app gets into a un-updatable state

RESOLVED FIXED in Firefox 36, Firefox OS v2.0

Status

Core Graveyard
DOM: Apps
--
critical
RESOLVED FIXED
4 years ago
9 months ago

People

(Reporter: amac, Assigned: amac)

Tracking

unspecified
mozilla36
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox34 wontfix, firefox35 wontfix, firefox36 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

(Whiteboard: [blocking][tef-triage])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
STR:
  1. Install an app from a store. The app *must* be privileged and have a specific origin (like app://whatever.com).
  2. Update the app on the store
  3. Check for updates and apply the update.
  4. Before the update is downloaded completely, remove the network from the device (stopping the wifi for example).

  5. At a later point (before it gives the network error), reactivate the network, and cancel the update.
  6. Resume the update

Expected:
  The update is applied correctly.

Actual: 
  Sometimes (It might depend on the exact moment when the download is stopped) the update cannot be applied. If the app is uninstalled, it cannot be installed again.

The uninstallable state can be fixed by:

  1. Uninstalling the app
  2. Executing 
adb shell rm -r /data/local/tmp/webapps
(Assignee)

Comment 1

4 years ago
[Blocking Requested - why for this release]:
This is failing on 2.0, and it's a problem that can/will happen for users and that for most of them leave the device on a state where some apps cannot be used anymore.
blocking-b2g: --- → 2.0?
Blocks: 1036490
Severity: normal → critical
Whiteboard: [blocking][tef-triage]
(Assignee)

Updated

4 years ago
Assignee: nobody → amac.bug
(Assignee)

Comment 2

4 years ago
Created attachment 8515111 [details] [diff] [review]
Patch V1. Don't calculate the downloaded file hash if we didn't download it
(Assignee)

Comment 3

4 years ago
Comment on attachment 8515111 [details] [diff] [review]
Patch V1. Don't calculate the downloaded file hash if we didn't download it

Now this was an interesting bug... I don't know how or why, sometimes when we're downloading a update if I remove the network I don't get a NETWORK_ERROR. Instead the file appears to be still downloading, but stuck at 0 bytes. If I cancel the download then (can't do anything else really) it seems we have updated the ETAG but nothing else (maybe the package was downloaded but not applied?). 

So when I try to resume the download, the server answers with a 304 (cause the etag is the same). And so we don't download anything but try to hash the (not downloaded) file. Which fails and gives and exception. And at that point, you can kiss that application goodbye forever until you do a hard reset (or do some file-level modifications).

In any case, the fix is simple, just don't try hashing the file if we got a 304. I've tested this and this allows me to use/and update again an app that was borked.

Test run is at: 
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=bf57bc219eb7
Attachment #8515111 - Flags: review?(fabrice)
(Assignee)

Comment 4

4 years ago
Oh, I also have the b2g32 patch ready (and tested). Will upload once this one is reviewed.
Attachment #8515111 - Flags: review?(fabrice) → review+
This should block as it presents a very broken experience to end users.

Antonio, can you please request appropriate uplift approvals for your patches?

Thanks
blocking-b2g: 2.0? → 2.0+
(Assignee)

Comment 6

4 years ago
Created attachment 8516482 [details] [diff] [review]
*Incorrect* B2G 32 patch, this is the M-C patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Some times apps will become unlaunchable/unupdatable/uninstallable when there's a network error. That state is non transient and requires a hard reset to get out of.
Testing completed: Locally
Risk to taking this patch (and alternatives if risky): Very low, it just alters the order of a couple of validations
String or UUID changes made by this patch: None

r=fabrice
This is the equivalent patch for B2G32 (the Webapps.jsm was moved and it's missing one line that's on Mozilla Central).
Attachment #8516482 - Flags: review+
Attachment #8516482 - Flags: approval-mozilla-b2g32?
(Assignee)

Comment 7

4 years ago
Comment on attachment 8515111 [details] [diff] [review]
Patch V1. Don't calculate the downloaded file hash if we didn't download it

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Some times apps will become unlaunchable/unupdatable/uninstallable when there's a network error. That state is non transient and requires a hard reset to get out of.
Testing completed: Locally
Risk to taking this patch (and alternatives if risky): Very low, it just alters the order of a couple of validations
String or UUID changes made by this patch: None
Attachment #8515111 - Flags: approval-mozilla-b2g34?
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #3)
> Comment on attachment 8515111 [details] [diff] [review]
> Patch V1. Don't calculate the downloaded file hash if we didn't download it
> 
> Now this was an interesting bug... I don't know how or why, sometimes when
> we're downloading a update if I remove the network I don't get a
> NETWORK_ERROR. Instead the file appears to be still downloading, but stuck
> at 0 bytes. If I cancel the download then (can't do anything else really) it
> seems we have updated the ETAG but nothing else (maybe the package was
> downloaded but not applied?). 
> 
> So when I try to resume the download, the server answers with a 304 (cause
> the etag is the same). And so we don't download anything but try to hash the
> (not downloaded) file. Which fails and gives and exception. And at that
> point, you can kiss that application goodbye forever until you do a hard
> reset (or do some file-level modifications).
> 
> In any case, the fix is simple, just don't try hashing the file if we got a
> 304. I've tested this and this allows me to use/and update again an app that
> was borked.
> 
> Test run is at: 
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=bf57bc219eb7

Hi Anthonia, this test run shows a lot of test failures like in the web-platform tests. Is this fixed/related to this change ?
Flags: needinfo?(amac.bug)
Keywords: checkin-needed
(Assignee)

Comment 9

4 years ago
I saw that but don't think it's related with the patch. The error seems to be related with launching the tests, and the only change this patch has is changing the order of a couple of checks when installing apps. Oh, and it's Antonio ;)
Flags: needinfo?(amac.bug)
(Assignee)

Comment 10

4 years ago
Re-requesting checkin. I didn't notice it was erased. As I said, the errors don't seem related at all with the patch changes.
Keywords: checkin-needed
(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #9)
> I saw that but don't think it's related with the patch. The error seems to
> be related with launching the tests, and the only change this patch has is
> changing the order of a couple of checks when installing apps. Oh, and it's
> Antonio ;)

I assume you pushed on top of b2g34? That'd be an expected result then :)
Attachment #8515111 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Attachment #8516482 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+
https://hg.mozilla.org/mozilla-central/rev/a24fb43555a8
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → fixed
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/1b076c5a760c

Your b2g32 patch doesn't apply...
status-b2g-v2.1: affected → fixed
status-firefox34: --- → wontfix
status-firefox35: --- → wontfix
status-firefox36: --- → fixed
Flags: needinfo?(amac.bug)
(Assignee)

Comment 15

4 years ago
Created attachment 8517714 [details] [diff] [review]
B2G32 patch. The correct one this time...

r=fabrice

This is the correct B2G32 patch... I uploaded the same patch twice the other morning.

Dunno if I should ask for approval again, not obsoleting the other one still because of that.
Flags: needinfo?(amac.bug)
Attachment #8517714 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #8516482 - Attachment description: B2G32 patch → *Incorrect* B2G 32 patch, this is the M-C patch
Comment on attachment 8516482 [details] [diff] [review]
*Incorrect* B2G 32 patch, this is the M-C patch

Marking a patch obsolete doesn't clear the flag. But it does make the attachments easier to follow.
Attachment #8516482 - Attachment is obsolete: true
Blocks: 1062883

Updated

9 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.