Closed Bug 887700 Opened 11 years ago Closed 11 years ago

[OTA] [Data Migration] [Email] Cannot load email in Inbox after update from v1.0.1 to v1.1.0

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, b2g18 verified, b2g-v1.1hd fixed)

VERIFIED FIXED
blocking-b2g leo+
Tracking Status
b2g18 --- verified
b2g-v1.1hd --- fixed

People

(Reporter: askeing, Assigned: iliu)

References

Details

(Keywords: dataloss, reproducible, Whiteboard: [migration])

Attachments

(6 files)

### ENV:
Inari
from v1.0.1 (pvt)
    Gaia:     93241eb6c5d6c110710fad8da3ccd4423312b0c9
    Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
    BuildID   20130624070215
    Version   18.0
to v1-train (complete gaia/gecko OTA update)
    Gaia mozillaorg - 16b872bd7d3aa4f1a7ae73bb19806af8b5d23dc0
    Gecko mozillaorg - 914243b75089d42b0672c8b01b7adf33568309f5

Unagi
from v1.0.1 pvt
    Gaia:     93241eb6c5d6c110710fad8da3ccd4423312b0c9
    Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
    BuildID   20130626070217
    Version   18.0
to v1-train
    Gaia mozillaorg - 4d6cbe49fdba32f72ec8bc6280e4782c250ab41e
    Gecko mozillaorg - 914243b75089d42b0672c8b01b7adf33568309f5

PS:
The command of building complete gaia/gecko OTA update package:
B2G_UPDATER=1 B2G_SYSTEM_APPS=1 MOZ_BUILD_DATE=XXX ANDROIDFS_DIR=XXX ./build.sh gecko-update-full
ref: https://wiki.mozilla.org/B2G/Updating


### STR:
1. Flash device to v1.0.1 build from pvt.
2. Change update URL.
3. Connect to Wifi.
4. Open Email app.
5. Add email account, receive email.
6. Install gaia/gecko complete update.
7. After auto-restart, open Email app.
8. Check the settings and email in Email app.

## Actual results:
No original email in "Inbox" after update.
Can see email in "All Mail".
Email still have original account settings.

## Expected results:
Email still have original account settings and can see original email.
Blocks: 885114
blocking-b2g: --- → leo?
Component: General → Gaia::E-Mail
Keywords: dataloss
Whiteboard: [migration]
v1.0.1 uses the same database version as v1-train at this time, so there really shouldn't be any migration happening at all.  Now, v1-train does use our database caching logic for improved startup time, and the cookie cache won't exist, so it could take longer for messages to appear, etc.

Can you provide adb logcat output as described on https://wiki.mozilla.org/Gaia/Email/RequiredBugInfo?
Keywords: qawanted
Askeing - Can you address comment 1?
Flags: needinfo?(fyen)
This is something we'd block the release on if it is reproducible.
blocking-b2g: leo? → leo+
I did some migration testing today, and here is what I found - not sure which types of email account the reporter set up, but I used both Gmail and Hotmail.

STR:
1. Started with 

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
Gaia   f000719da5116eca00b78aa8121f38264d3dafee
BuildID 20130628070214
Version 18.0

2, Created two email accounts, one Hotmail and one Gmail. Load each account and confirm emails are seen in Inbox.
3. Changed the channel to nightly and performed an OTA to 1.1
4. Examined my email accounts.

Results:

Gmail account inbox shows as expected, with all message
Hotmail account shows an empty inbox with "no mail in this folder." Refreshing the account does nothing.

Andrew: unfortunately I forgot to enable the logging before I performed the OTA, but I will repeat the steps and get a log.
Keywords: qawanted
Thanks for the log and testing this.  The sitch:

- On v1.0.1, ActiveSync did not correctly initialize header.{to/cc/bcc} to null.  It was still initializing it on 'body', although the assignments correctly happened on the header.  This was fixed in 848266 which is on v1-train but not v1.0.1.

- v1.0.1 and v1.1 share the same database version so no schema upgrade happens so this matters.

- v1.1's contact resolution logic goes to access header.bcc and gets undefined.  We have a guard in resolvePeeps that bails if "=== null" , but that by definition does not fire on undefined.  I should probably stop using "===" with null, at least for guard cases.


The possible solutions to fix this migration problem:

- Make resolvePeeps use '==' instead of '==='.  This seems like a reasonable change in general.

- Rev the db version on v1-train (and master) so we discard all message data on upgrade from v1.0.1.  This has a certain appeal to it since it nukes all synchronized message data and rules out all bugs of these classes.  It may be overkill, however, since we've been pretty good about revving database versions.  In the case of bug 848266 we really should have revved the schema, but it seemed minor at the time.
Flags: needinfo?(fyen)
What's the risk involved in a db version rev at this stage?  Dylan can you weigh in on the risks of the two proposals in comment 7 and help make the engineering call & assign someone to this issue?
Flags: needinfo?(doliver)
Keywords: reproducible
A DB rev causes us to nuke our database and re-create it.  The sequence of events goes something like the following:

- In onupgradeneeded (this happens in a transaction):
-- retrieve the current configuration data and save it off as 'carryover'
-- drop all object stores, create currently used object stores
- In MailUniverse's startup logic:
-- Go through the list of accounts in the 'carryover' and call special recreateAccount methods that exist after dynamically loading the account modules for the account
-- The calls to recreateAccount cause the account to be saved.


Key things to note:
- The loading of the existing config and the creation of the new account details occur in separate database transactions.  So there is inherently a risk of losing all database state in the migration.  HOWEVER...
- We only propagate account information; any state that was not persisted to the server gets lost.  In practice, for v1.0.1 -> v1.1 this is not a giant deal because:
-- local drafts are not supported on v1.0.1 (I did uplift the back-end code for sanity reasons, but the UI does not support drafts)
-- more complicated enqueued account jobs/operations like message moves currently are discarded at startup because we punted on that
-- this leaves flagging a message or marking it as read as the state information that can get lost, but these are arguably minor things.
-- The worst-case scenario is that the user has to re-create their accounts
- Since the localdrafts logic is already in the v1.0.1 back-end, there are no invariant problems relating to the lack of a localdrafts folder on upgrade; it's already there.
- The flip-side of this risk is that by getting rid of enqueued jobs/operations we might also luck out and discard some state that was breaking us.  But as long as the user can remove the account and add it again, they can do it themselves.


The decision at this point for db rev'ing I think would hinge on whether we think there is body-fetch related state that could be inconsistent between v1.0.1 and v1.1 in a way that could break things.  I just skimmed our commit log and I don't see any bugs that (other than the activesync slipstream fix noted) that we landed but failed to uplift that would jump out at me as dangerous.


I'll file a bug on enhancing our migration logic in the future for when we have more data at-risk.  (We always knew we'd eventually need to do that.  This is not that case, but it's good to have it on the books.)
(In reply to Andrew Sutherland (:asuth) from comment #9)
> I'll file a bug on enhancing our migration logic in the future for when we
> have more data at-risk.  (We always knew we'd eventually need to do that. 
> This is not that case, but it's good to have it on the books.)

I filed bug 888955 on this.  Again, future-work, no impact on this bug.
Discussed with Andrew and settled on the simpler solution proposed on comment #7: 

> Make resolvePeeps use '==' instead of '==='.  
> This seems like a reasonable change in general.
Flags: needinfo?(doliver)
Assignee: nobody → bugmail
Either :evandx or :ianliu are going to take care of this one per discussion at the productivity team meeting this week.
Assignee: bugmail → iliu
I realize my comments were pretty long, so the end result is that the fix to be made is to change the === comparison to == in resolvePeeps in mailapi.js:

https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/data/lib/mailapi/mailapi.js#L528


And then a manual verification should be performed after making the fix.  It's not feasible to implement automated test coverage for this at this time.
Status: NEW → ASSIGNED
Update STR.

### ENV:
Unagi v1.0.1
  Gaia:     054cdc27404e2daca91d3065d9783681032b2151
  Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
  BuildID   20130716070209
  Version   18.0
to v1.1.0 nightly
  Gaia:     fb9362d34260771d4a00b9a0e10a6bbad397bd3b
  Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18/rev/629020cf576b
  BuildID   20130716070204
  Version   18.0

Inari v1.0.1
  Gaia:     054cdc27404e2daca91d3065d9783681032b2151
  Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
  BuildID   20130716070209
  Version   18.0
to v1.1.0 nightly
  Gaia:     fb9362d34260771d4a00b9a0e10a6bbad397bd3b
  Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18/rev/629020cf576b
  BuildID   20130716070204
  Version   18.0

### STR:
1. Flash device to v1.0.1 build from pvt.
2. Change update URL to v1.1.0 nightly.
3. Connect to Wifi.
4. Open Email app.
5. Add ActiveSync email account (manual setup), receive email.
   (ex: Email: gmail account
        Account type: ActiveSync
        Hostname: m.google.com
        Username: gmail account)
6. Install OTA update.
7. After auto-restart, open Email app.
8. Check the settings and email in Email app.

## Actual results:
No original email in "Inbox" after update.
No original email in "[Gmail] > All Mail" after update.
Still have original account settings.

## Expected results:
Email still have original account settings and can see original email.
(In reply to Askeing Yen[:askeing] from comment #14)
> 5. Add ActiveSync email account (manual setup), receive email.
>    (ex: Email: gmail account
>         Account type: ActiveSync
>         Hostname: m.google.com
>         Username: gmail account)

This is a separate issue from the migration bug, but using gmail via ActiveSync is not supported and definitely should not be used when testing the e-mail app.  (Gmail's ActiveSync implementation has various horrible issues.)  I've filed bug 894785 to blacklist Google's ActiveSync implementation.
(In reply to Andrew Sutherland (:asuth) from comment #15)
> This is a separate issue from the migration bug, but using gmail via
> ActiveSync is not supported and definitely should not be used when testing
> the e-mail app.  (Gmail's ActiveSync implementation has various horrible
> issues.)  I've filed bug 894785 to blacklist Google's ActiveSync
> implementation.
Thanks for your information.

Update STR again.
### STR:
1. Flash device to v1.0.1 build from pvt.
2. Change update URL to v1.1.0 nightly.
3. Connect to Wifi.
4. Open Email app.
5. Add ActiveSync email account (ex: hotmail), receive email.
6. Install OTA update.
7. After auto-restart, open Email app.
8. Check the settings and email in Email app.

## Actual results:
No original email in "Inbox" after update.
Still have original account settings.

## Expected results:
Email still have original account settings and can see original email.
Ian, can you provide a status update on this one?
Flags: needinfo?(iliu)
Dylan,
I use some time to review Bocoup's patch and HD+ issue. But I still feel sorry to reply the status late. I was working to update build version from v1.0.1 to v1-train with Askeing. We try to overcome updating issue on QA's tool manually.(https://github.com/Mozilla-TWQA/B2G-flash-tool/blob/master/change_OTA_URL.sh) Now, we can use the tool on ubuntu and Mac.

Then, I have try to modify qual sign according to comment 13. But it still not works for me.

My steps are as following:
1.  Flash device to v1.0.1 build from pvt.
2.  Change update URL to v1.1.0 nightly.
3.  Connect to Wifi.
4.  Open Email app.
5.  Add ActiveSync email account (ex: gmail), receive email.
*6. Revise code via comment 13. Then, rebuild email library(make install-into-gaia).
7.  Install OTA update.
*8. After auto-restart, re-install email app.(make install-gaia APP=email)
9.  Open Email app.
10. Check the settings and email in Email app.

I think I should check the account with hotmail later.
Flags: needinfo?(iliu)
Attached file ActiveSync@hotmail.com
The modification does not work for hotmail account. I will need to trace the root cause deeply. Andrew, does the error log like to your expected result? If you have resource, please help to take a look for the log. Thank you so much.
Attachment #781839 - Flags: feedback?(bugmail)
(Just so we're clear, per comment 15, do not try and use gmail for ActiveSync purposes.  hotmail.com/outlook.com really is the preferred implementation to test against, but exchange accounts would also be okay.)

Ian, can you put up a pull request with the modified code you were testing with?  The log from your run looks like it doesn't have the pached code.

Specifically, the code right now looks like:

  resolvePeeps: function(addressPairs) {
    if (addressPairs === null)
      return null;
    var resolved = [];
    for (var i = 0; i < addressPairs.length; i++) {
      resolved.push(this.resolvePeep(addressPairs[i]));
    }
    return resolved;
  }

In the ActiveSync case, resolvePeeps(undefined) will get called.  The error message is consistent with that undefined value reaching the for loop.

If the code is changed to:

  resolvePeeps: function(addressPairs) {
    if (addressPairs == null) // THIS LINE CHANGED
      return null;
    var resolved = [];
    for (var i = 0; i < addressPairs.length; i++) {
      resolved.push(this.resolvePeep(addressPairs[i]));
    }
    return resolved;
  }

Then the if clause should trigger since (undefined == null) and the function should bail, returning null.
Flags: needinfo?(iliu)
Attachment #781839 - Flags: feedback?(bugmail)
The WIP is including of console.log for verification.

Andrew,
I'm not able to flash(make install-gaia) the revised patch to device after finished OTA update on v1-train. So, Askeing and I will make a build which is including of the WIP and aim the specific OTA.
Flags: needinfo?(iliu)
(In reply to Ian Liu [:ianliu] from comment #21)
> I'm not able to flash(make install-gaia) the revised patch to device after
> finished OTA update on v1-train. So, Askeing and I will make a build which
> is including of the WIP and aim the specific OTA.

Like "adb devices" fails to list the device because "remote debugging" gets disabled after the update and needs to be turned on?  Or something else?
I try to flash the gaia into unagi with pvt build.
The pvt build has original apps locate at /system/b2g/webapps, and "make install-gaia" will push apps into /data/local/webapps.
It should be the root cause of c21.

After tracing the gaia Makefile, I found the "PRODUCTION=1 make install-gaia" can work fine (in my environment).
E/GeckoConsole(  886): [JavaScript Warning: "Expected pseudo-element but found '-moz-placeholder'.  Ruleset ignored due to bad selector." {file: "app://email.gaiamobile.org/shared/style/input_areas.css" li
ne: 40}]
E/GeckoConsole(  886): [JavaScript Warning: "Error in parsing value for '-moz-transition-delay'.  Declaration dropped." {file: "app://email.gaiamobile.org/style/mail.css" line: 51}]
E/GeckoConsole(  886): [JavaScript Warning: "Error in parsing value for 'transition'.  Declaration dropped." {file: "app://email.gaiamobile.org/style/mail.css" line: 55}]
E/GeckoConsole(  886): [JavaScript Warning: "Error in parsing value for 'background-image'.  Declaration dropped." {file: "app://email.gaiamobile.org/style/mail.css" line: 109}]
E/GeckoConsole(  886): [JavaScript Warning: "Error in parsing value for 'height'.  Declaration dropped." {file: "app://email.gaiamobile.org/style/mail.css" line: 161}]
I/IdleService(  742): Get idle time: time since reset 1023 msec
I/IdleService(  742): Idle timer callback: current idle time 1023 msec
I/IdleService(  742): next timeout 298975 msec from now
I/IdleService(  742): SetTimerExpiryIfBefore: next timeout 298975 msec from now
I/IdleService(  742): reset timer expiry to 298985 msec from now
I/IdleService(  742): Idle timer callback: tell observer 47e37590 user is idle
I/IdleService(  742): Get idle time: time since reset 1025 msec
I/Gecko   (  886): TCPSocket: window init: 4
I/GeckoDump(  886): ERR: onerror reporting: ReferenceError: define is not defined @ app://email.gaiamobile.org/gaia_build_defer_index.js : 484
E/GeckoConsole(  886): [JavaScript Error: "ReferenceError: define is not defined" {file: "app://email.gaiamobile.org/gaia_build_defer_index.js" line: 484}]
Andrew,
Email app is not able to launch after flashed with the revised patch. The following modified files on v1-train after we done "make install-into-gaia" in email-lib repository.

#	modified:   apps/email/js/ext/mailapi/activesync/configurator.js
#	modified:   apps/email/js/ext/mailapi/activesync/protocollayer.js
#	modified:   apps/email/js/ext/mailapi/chewlayer.js
#	modified:   apps/email/js/ext/mailapi/composer.js
#	modified:   apps/email/js/ext/mailapi/composite/configurator.js
#	modified:   apps/email/js/ext/mailapi/imap/probe.js
#	modified:   apps/email/js/ext/mailapi/main-frame-setup.js
#	modified:   apps/email/js/ext/mailapi/worker-bootstrap.js

Any idea for "app://email.gaiamobile.org/gaia_build_defer_index.js" line: 484 ?
Our work on 'master' has diverged sufficiently from what is on v1-train that you should either just directly modify the built/concatenated file in gaia/v1-train or establish a v1-train branch of gaia-email-libs-and-more that's roughly consistent with what is already on v1-train.  (Additional fix-up commits would likely be required.)

I personally would probably just directly modify the file in question in gaia/v1-train since there's very little else we should end up uplifting.
Andrew,

I have a test for master/v1-train without OTA updated. Because we have made sure that there is  no different on OTA vs. upgrade Email app only. Both of them work fine with the patch. 

My steps are as following:

1.  Flash device to master/v1.0.1 build from pvt.
2.  Connect to Wifi.
3.  Open Email app.
4.  Add ActiveSync email account (ex: hotmail) manually, receive email.
5.  According comment 13, revise code in Email app of gaia repository directly.(skip to rebuild email-libary)
6.  Flash Email app with the modification.(PRODUCTION=1 make install-gaia APP=email)
7.  Open Email app.
8.  Check the settings and email in Email app.

Evan if the above steps are working fine, we still need to make sure with OTA updated.
Attachment #782656 - Flags: review?(bugmail)
Comment on attachment 782656 [details]
Pointer to Github pull request 229.html

r=asuth.

To be absolutely clear, we should end up with 3 landings occurring:
1) the change to the actual mailapi.js in gaia-email-libs-and-more against gaia-email-libs-and-more/master
2) the derived change from that against gaia/master done using "make install-into-gaia" where we make sure that the only change is what's here in this pull request.
3) the gaia-only change against gaia/v1-train

I think the pull request you're making is #2?  If so, please also make #1 and check whether #2 successfully cherry-picks for #3 so auto-uplift works; if not, manually prepare and land the change for v1-train!

Thanks!

(And I'm assuming the OTA testing you are talking about is something you or QA will manually verify once the v1-train nightly has the fix in it?)
Attachment #782656 - Flags: review?(bugmail) → review+
(In reply to Andrew Sutherland (:asuth) from comment #28)
> Comment on attachment 782656 [details]
> Pointer to Github pull request 229.html
> 
> r=asuth.
> 
> To be absolutely clear, we should end up with 3 landings occurring:
> 1) the change to the actual mailapi.js in gaia-email-libs-and-more against
> gaia-email-libs-and-more/master
> 2) the derived change from that against gaia/master done using "make
> install-into-gaia" where we make sure that the only change is what's here in
> this pull request.
> 3) the gaia-only change against gaia/v1-train
> 
> I think the pull request you're making is #2?  If so, please also make #1
> and check whether #2 successfully cherry-picks for #3 so auto-uplift works;
> if not, manually prepare and land the change for v1-train!
> 
> Thanks!
> 
> (And I'm assuming the OTA testing you are talking about is something you or
> QA will manually verify once the v1-train nightly has the fix in it?)

Please needinfo? william hsu or askeing from tw-qa team if you need this tested.  they know how to run the OTA testing once this patch lands.
(In reply to Andrew Sutherland (:asuth) from comment #28)
> Comment on attachment 782656 [details]
> Pointer to Github pull request 229.html
> 
> r=asuth.
> 
> To be absolutely clear, we should end up with 3 landings occurring:
> 1) the change to the actual mailapi.js in gaia-email-libs-and-more against
> gaia-email-libs-and-more/master
> 2) the derived change from that against gaia/master done using "make
> install-into-gaia" where we make sure that the only change is what's here in
> this pull request.
> 3) the gaia-only change against gaia/v1-train
For the section #3, you mean that using the gaia-only change which is built from #1 + #2 for working gaia/v1-train. And I just use "git stash" the modified files(#2) at gaia/master. Then, I do "git stash pop" to gaia/v1-train. After the process, I have done the test with comment 27 steps. It works for me.
> 
> I think the pull request you're making is #2?  If so, please also make #1
> and check whether #2 successfully cherry-picks for #3 so auto-uplift works;
> if not, manually prepare and land the change for v1-train!
It's pretty clear for me. In fact, we only have the branch of gaia-email-libs-and-more/master regarding to gaia/master, gaia/v1-train, gaia/v1.0.1 . (I see other branches(demo, es5-js-only, .. ) in gaia-email-libs-and-more. I might know them in the future.)
> 
> Thanks!
> 
> (And I'm assuming the OTA testing you are talking about is something you or
> QA will manually verify once the v1-train nightly has the fix in it?)
Yes, Askeing and I were trying to how we can verify the fixing patch.:)
(In reply to Tony Chung [:tchung] from comment #29)
> (In reply to Andrew Sutherland (:asuth) from comment #28)
> > Comment on attachment 782656 [details]
> > Pointer to Github pull request 229.html
> > 
> > (And I'm assuming the OTA testing you are talking about is something you or
> > QA will manually verify once the v1-train nightly has the fix in it?)
> 
> Please needinfo? william hsu or askeing from tw-qa team if you need this
> tested.  they know how to run the OTA testing once this patch lands.
Tony,
Thanks for your reminder. Askeing is very enthusiastic to give some assistance.
Andrew,
Please help to review the patch(section#2) again. Thanks.
Attachment #783048 - Flags: review?(bugmail)
Attachment #783048 - Flags: review?(bugmail) → review+
landed in gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/229
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/3832afd4fe675aec0a41e9e03cf4e821247e9b09

landed in gaia/master:
https://github.com/mozilla-b2g/gaia/pull/11246
https://github.com/mozilla-b2g/gaia/commit/2d9ee5afaaa49d6f2c3db6971b874862fbbbd66c

The extra change in the gaia/master fix is a correction of my accidental backing out of bug 897308 owing to a git submodule mismatch that I did not catch when I landed my fix for bug 813411.  Uplift of that extra change to v1.1 is fine and safe.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Uplifted 2d9ee5afaaa49d6f2c3db6971b874862fbbbd66c to:
v1-train: 03eb6172a2b45ba8333e23a08dc1b66b7ef91999
Unagi pvt v1.0.1
  Gaia:     054cdc27404e2daca91d3065d9783681032b2151
  Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
  BuildID   20130729070220
  Version   18.0
to v1.1.0 http://update.boot2gecko.org/unagi/1.1.0/nightly/update.xml
  Gaia:     ddb922ed002e88d01f71199da32ff75911b455b2
  Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18/rev/ba71c64705e5
  BuildID   20130801101655
  Version   18.0

Verified.
v1.1.0hd: 03eb6172a2b45ba8333e23a08dc1b66b7ef91999
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: