Closed Bug 796814 Opened 7 years ago Closed 7 years ago

[email] Email shows a black-screen on startup if there was a network problem when creating the account and there are no known folders. Remove assertion and automatically resync.

Categories

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

defect

Tracking

(blocking-basecamp:+)

RESOLVED FIXED
blocking-basecamp +

People

(Reporter: ghtobz, Assigned: asuth)

References

Details

(Keywords: dogfood, Whiteboard: [QARegressExclude],)

Attachments

(1 file)

[GitHub issue by nhirata on 2012-10-01T16:51:25Z, https://github.com/mozilla-b2g/gaia/issues/5587]

## Environment :
Otoro phone, build 2012-10-01 us
Taken from default.xml in b2g-distro: 
* "platform_build" revision= 795261940c8b11fb7dddd7a8e9dd8561fdc4fb64
* "gaia" revision= 5feb07b511ab907942731fc4961ccbce3958d021 
* "releases-mozilla-central" revision= 80b40bba7f30f6cea5cb148cbbdcbf008691972b
* "gonk-misc" revision= 202d2c62443c098b125e5d9d7b42226d98230e44
  
## Repro :
1.launch email for the first time.
2. setup hotmail account and gmail account
3. continue to mail

## Expected :
1. email launches and you are in the mail view

## Actual :
1.blackscreen shows up

## Note :
10-01 09:48:50.390: I/GeckoDump(996): [31mERR: FATAL: We have an account without an inbox![0m

Full logcat : https://gist.github.com/3812974
[GitHub comment by mozsquib on 2012-10-01T18:48:30Z]
That logcat is missing an awful lot of logging that I'd expect to be there. I'm honestly not sure how you even managed to create an account in that case.
[GitHub comment by mozsquib on 2012-10-01T18:50:20Z]
@asutherland, do `console.log` statements inside GELAM go into logcat?
[GitHub comment by asutherland on 2012-10-01T19:23:46Z]
@mozsquib They should, and indeed I see "Synced folder list" in there twice after the explosions, which are ActiveSync things.  Perhaps we have introduced some type of race between the folder list being initially populated and the UI showing the contents of the folder?
[GitHub comment by mozsquib on 2012-10-01T19:53:06Z]
@asutherland That's true. This doesn't look like the logcat from the first run though. There's no logging for the autoconfiguration process, and the "Synced folder list" isn't preceded by any folders being added.
[GitHub comment by mozsquib on 2012-10-01T23:52:45Z]
@asutherland, it's theoretically possible for us to get into a state with no inbox even when there aren't other bugs happening. It certainly won't be very common, but maybe we should change things so we don't barf when this happens?
(In reply to GH to BZ from comment #5)
> [GitHub comment by mozsquib on 2012-10-01T23:52:45Z]
> @asutherland, it's theoretically possible for us to get into a state with no
> inbox even when there aren't other bugs happening. It certainly won't be
> very common, but maybe we should change things so we don't barf when this
> happens?

Do you mean accessing an account without an inbox?  That seems like the type of thing that would only happen for a shared-folders-only account, and that's arguably not remotely one of our current use-cases.  (And we probably would have other deficiencies, like not taking advantage of IMAP namespaces properly, etc.)

I think this is a case where the assertion and fast-fail are helping us see that some kind of bug is happening, which is the right thing.  Otherwise, we potentially end up with a different bug when we have no folders that we can select, or we regress and no longer see the inbox as an inbox and instead select the alphabetically first folder, which could be very wrong, etc.
Assignee: nobody → bugmail
blocking-basecamp: ? → +
QA Contact: nhirata.bugzilla
Component: Gaia → Gaia::E-Mail
Assignee: bugmail → nobody
(In reply to Andrew Sutherland (:asuth) from comment #6)
> Do you mean accessing an account without an inbox?  That seems like the type
> of thing that would only happen for a shared-folders-only account, and
> that's arguably not remotely one of our current use-cases.

Among other things, this could happen if your network connection dies after you create the account, but before syncFolderList fires the callback, and then you restart the app. In fact, this is probably how this bug manifested itself, since we seem to have strange network hiccups on the actual phone...
Makes sense; then we could compel a syncFolderList at startup if our list of folders is empty.

UI options to deal with the fact that we don't have a folder to show are:
1) Show the spinny "I am setting up your account" page if we have to do this.
2) Have the app sorta block and take a really long time to startup until the issue is resolved.

I guess #1 is the humane option...


Plan B would be to not let the account be created or at least persisted until syncFolderList has run.  That might be more troublesome because if we have an existing account doing things in the background, we could end up sad if it causes persistence or we try and stop it from persisting things that invariants require really get persisted.
We also need to figure out when to update the folder list after the initial setup. Right now, I have some hacky code in ActiveSync to do this, but a better solution would be nice. Maybe we can do something here to help with that? Even simply calling syncFolderList at startup every time would be ok.
Cody, Jim suggested this one as another good bug for getting started in the email component.
Assignee: nobody → cbrocious
Jim, we should never setup anything as part of the initial setup since folders can always change. Instead, check folders and adjust when sync-ing. The setup should be mostly stateless. On a related note, my gmail setup is still busted. I get stuck at the spinner. Please remove all state and data fetching from the setup.
(In reply to Andreas Gal :gal from comment #11)
> Jim, we should never setup anything as part of the initial setup since
> folders can always change.

We update the folder list every time the mail app starts (for ActiveSync, not sure about IMAP).

> Please remove all state and data fetching from the setup.

How does that benefit us? If we fail to sync the folder list, we can't sync any messages anyway. Besides, the account actually gets created internally before we start syncing the folders. The only thing that we could change would be to show a different progress UI after we've created the account but before we've synced the folder list. It wouldn't do anything to fix setup for you.
IMAP currently does not call syncFolderList at startup, although we obviously need to.  The key ingredients there are to:
1) try and avoid increasing the time-to-first-up-to-date-messages display
2) store when we last checked and use that to suppress the check so we can avoid pathological bandwidth wasting
3) try and avoid gratuitously spinning up an extra connection. For IMAP this requires a little thought just because right now we leave connections in the folder even when no sync is active so they can IDLE, although we don't do anything with that IDLE.  The IMAP unit tests make some weak assumptions about this in terms of when new connections are created and torn down.

In terms of Andreas' problem, I rather strongly doubt that syncFolderList is what is broken.  An "adb logcat | grep Gecko" would be really helpful, at least on a profile where:
  user_pref("browser.dom.window.dump.enabled", true);

Having said that, since both ActiveSync and IMAP guarantee that a folder exists, and the server name is a given (INBOX for IMAP, zero-padded 1 for ActiveSync), we can declare victory earlier as Andreas asks.  The folder list may just be basically empty apart from INBOX for a few seconds if things are going well, and eternally empty if things are hosed.
Duplicate of this bug: 804321
MoCo account worked like a charm for me in the 10-18 stable build, but hosed in the 10-24 build.  The update can't possibly touch /data, so I have no idea what changed.  Maybe being offline briefly can cause this?
Severity: normal → critical
Priority: -- → P1
(In reply to Chris Jones [:cjones] [:warhammer] from comment #15)
> MoCo account worked like a charm for me in the 10-18 stable build, but hosed
> in the 10-24 build.  The update can't possibly touch /data, so I have no
> idea what changed.  Maybe being offline briefly can cause this?

Not sure if you saw/read the e-mails I sent to dev-gaia end of 10/23 (Pacific), but the answer is that you were probably offline when you ran the e-mail app after upgrading.  The migration process propagated the account credentials but not the folders, which triggers this bug.

I'm not sure what Cody's progress is on this, but I can provide a quick fix approximating what I suggested at the end of comment 13 (which is also in line with what Andreas requests).  This will result in the new (temporary) failure case of "I only have an INBOX and there's nothing in it!", but that will be much more recoverable by forcing the app closed and re-opening it.  And the various improved offline/error-handling bugs can then eliminate that or make it more obvious to the user why things are not great (they are offline!).

To try and make the folder list synchronizing happen as quickly as possible once we go online (and avoid adding additional logic), we probably want to schedule an invocation of it as a non-persisted operation, forcing it to be the first thing in the queue for the account.  This also nets automatic retry functionality and the improvements it gets.
(In reply to Andrew Sutherland (:asuth) from comment #17)
> I'm not sure what Cody's progress is on this, but I can provide a quick fix
> approximating what I suggested at the end of comment 13 (which is also in
> line with what Andreas requests).  This will result in the new (temporary)
> failure case of "I only have an INBOX and there's nothing in it!", but that
> will be much more recoverable by forcing the app closed and re-opening it. 
> And the various improved offline/error-handling bugs can then eliminate that
> or make it more obvious to the user why things are not great (they are
> offline!).

I haven't begun work on this bug yet -- it's next on my list and I'll be able to start on it tomorrow at the latest.  However, I can certainly take on others if you want to tackle this one.
Re-assigning to Andrew since he already has a good read one this one.
Assignee: cbrocious → bugmail
(In reply to Andrew Sutherland (:asuth) from comment #17)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #15)
> > MoCo account worked like a charm for me in the 10-18 stable build, but hosed
> > in the 10-24 build.  The update can't possibly touch /data, so I have no
> > idea what changed.  Maybe being offline briefly can cause this?
> 
> Not sure if you saw/read the e-mails I sent to dev-gaia end of 10/23
> (Pacific), but the answer is that you were probably offline when you ran the
> e-mail app after upgrading.  The migration process propagated the account
> credentials but not the folders, which triggers this bug.
> 

Didn't see.  I most likely was offline, because of bug 805548.
Status: NEW → ASSIGNED
Summary: [email] Email cannot launch if it does not think that the account does not have an inbox. → [email] Email shows a black-screen on startup if there was a network problem when creating the account and there are no known folders. Remove assertion and automatically resync.
Duplicate of this bug: 806441
Whiteboard: [label:email] → [label:email], [dogfood-blocker]
There hasn't been much movement here Andrew, yet this is a P1/critical bug affecting dogfooders. Can you take a look at this asap?
The pull request I've been working this on is:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/72

Should be up for review today.
Comment on attachment 677984 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/72

Check it, I installed dietrich's funky add-on and now I make fancy review requests too.
https://addons.mozilla.org/en-US/firefox/addon/github-tweaks-for-bugzilla/
Attachment #677984 - Flags: review?(squibblyflabbetydoo)
Attachment #677984 - Flags: review?(squibblyflabbetydoo) → review+
Landed on gaia/master with merge:
https://github.com/mozilla-b2g/gaia/commit/018c73c509157a6bea17501b186a8c7ff4315efa
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Duplicate of this bug: 808833
Keywords: dogfood
Whiteboard: [label:email], [dogfood-blocker] → [label:email],
Reviewed and verified 
on build ID:20121231070201
Device "Unagi"
OS "Gonk"
Status: RESOLVED → VERIFIED
Status: VERIFIED → RESOLVED
Closed: 7 years ago7 years ago
Works fine on Unagi build 20130102070202.
Whiteboard: [label:email], → [QARegressExclude],
not going to create a specific moztrap test case for this. It'll be covered by the various account creation test cases already present.
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.