796444 - [email/activesync] ActiveSync does not automatically retry or report account problems.

Status: RESOLVED WORKSFORME

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

Description

[GH to BZ]

[GitHub issue by nhirata on 2012-07-30T21:07:03Z, https://github.com/mozilla-b2g/gaia/issues/2974]
## Environment :
Otoro phone, build 2012-07-30
Taken from default.xml in b2g-distro: 
* "platform_build" revision= 2163d79af8c06ffcf7607a83e01dc5bf1107fd8e
* "gaia" revision= 759caf5aa151ce06ce580e0a9ddb6ffe24ff8334
* "mozilla-central" revision= 07226cfa1211b2f8fc7702345a7184fae7c3962b
* "gonk-misc" revision= c6a9a256cd4a1c53696488739d36778b9dbfc881

## Repro :
1. setup the wifi with a bad username/password for a wpa connection
2. launch email app
3. setup with hotmail account

## Expected :
* time out since there's no network connection; or an early warning, no network connection


## Actual :
* just waits...

## Note :
* logcat:

Comment 1

[GH to BZ]

[GitHub comment by mozsquib on 2012-09-27T19:45:18Z]
I believe this will be fixed by PR #5325. Can you retest once that's merged?

Comment 2

[GH to BZ]

[GitHub comment by mozsquib on 2012-09-27T22:39:14Z]
Ok, that PR is checked in now, so you should be able to test this out now.

Comment 3

[Jim Porter (:squib)]

asuth, what do you think about this:

Set an initial timeout for all XHRs to 10 seconds. Every time we receive a Progress event, update the timeout to be 5 seconds in the future. I think this should be pretty straightforward, since we just need to keep track of when the XHR started, and then use the Progress event's timestamp to figure out how much time has elapsed (the spec explicitly allows us to update the timeout in the middle of a request). This way, we can automatically adapt to large and small requests/responses coming through.

We might also want a cap on the timeout for sanity. Say 5 minutes?

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

This sounds very reasonable to me.  I think I would avoid the maximum cap logic for now.  I'm not so concerned about the potential for 'jumpy NTP clock problems' since Date.now is not monotonically increasing, just the extra edge cases.  That's something we can do later.  And of course let's try and avoid this being a foot-gun when we adopt push notifications :)

Comment 5

[Jim Porter (:squib)]

Push notifications will naturally have their own logic for timeouts (probably just a constant, large value). Luckily, the responses for push notifications are basically guaranteed to be very small.

Comment 6

[Jim Porter (:squib)]

Oh, and for working around the fact that time is a world of pain, we could just make sure we start with a reasonably-long timeout (10-20 seconds is probably plenty) and make sure never to decrease the accumulated timeout value. Then, occasional hiccups with the timestamp won't immediately kill our connection.

Comment 7

[Jim Porter (:squib)]

I guess another option would be to use setTimeout and continually cancel and restart the setTimeout callback as we get progress notifications...

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

Yeah, I thought you were proposing to just clearTimeout on every progress and do a new setTimeout.  Since that's all relative time, jumpy time is not our problem in that case (and I think the right NSPR stuff happens under the hood.)

Comment 9

[Jim Porter (:squib)]

I should clarify then: I was originally thinking about using the |timeout| attribute on the XHR, which lets you specify the timeout in milliseconds. We could just keep adding to that timeout every time we got a Progress event from the XHR, but that would (I think) involve using clock time.

Comment 11

[Dylan Oliver [:doliver]]

The title of this one is about Activesync but will the fix serve as a more general solution to handling server communication issues in the email app?

Comment 12

[Jim Porter (:squib)]

It'll probably fix some of the setup issues, but other than that, I have no idea.

Comment 13

[Jim Porter (:squib)]

Partially fixed in: https://github.com/mozilla-b2g/gaia/pull/6447
                    https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/76

Leaving this bug open so that we can also add some retry/backoff code to ActiveSync (like IMAP already has).

Comment 14

[Jim Porter (:squib)]

asuth: Do you think it makes sense to allow the user to abort the XHRs in the autoconfig process as a part of this bug? It broadly falls under the scope of what comment 0 is talking about, but it's not exactly the same. Mainly, this would have the benefit of saving network resources and preventing possible breakage if the user hit "back" when trying to autoconfig (e.g. if they realized they typed their email address in wrong).

Comment 15

[Jim Porter (:squib)]

Actually, I guess that needs to wait until I hear back from Andreas about what he'd like us to do with making setup non-blocking.

Comment 16

[Dietrich Ayala (:dietrich)]

Been almost 2 weeks - what needs to happen in this bug?

Comment 17

[Jim Porter (:squib)]

(In reply to Dietrich Ayala (:dietrich) from comment #16)
> Been almost 2 weeks - what needs to happen in this bug?

I'm probably going to allow canceling the requests during autoconfigure, since it's easy, and we also want to be able to retry failed attempts to talk to the server for ActiveSync. IMAP already supports this, so it should be straightforward to make ActiveSync do the same.

Comment 18

[Alex Keybl [:akeybl]]

The user impact here seems fairly minimal, and should be uncommon - marking as a soft blocker.

Comment 19

[ayman maat :maat]

no updates on this bug for some time. RFI to Jim to enquire what the current status of the bug is.

Comment 20

[Dylan Oliver [:doliver]]

Ayman, this bug was marked blocking- on 11/30 so it's not getting any attention right now.

Comment 21

[ayman maat :maat]

(In reply to Dylan Oliver [:doliver] from comment #20)
> Ayman, this bug was marked blocking- on 11/30 so it's not getting any
> attention right now.

Thanks for the response Dylan. On what grounds could i ask was it defined as not blocking? From a UX perspective if the app enters an error state due to loss of connection and the user is not informed of the error, and therefore cannot identify the error and recover from it this is a blocking bug by definition.

RFI to josh to bring this bug back into triage.

Comment 22

[Andrew Sutherland [:asuth] (he/him)]

I somewhat recently landed an additional error handling/feedback patch that built on Jim's error/timeout improvements in comment 13.  We should now display an error during synchronization with a status banner thing across the bottom that offers the ability to retry.

There is, however, no automatic retry capability or connection status tracking.  Because ActiveSync doesn't rely on a long-lived connection, the retries are somewhat less important since there's less chance of a connection 'going stale' since every request is a new XHR.  However, it would be useful in that it could help us more confidently report that we seem to be having trouble connecting to a server.  And then once we know that is happening, like IMAP, we can stop retrying until we have an indication the server is back.

Comment 23

[Josh Carpenter [:jcarpenter]]

Changing needsinfo to Casey, who is the UX owner for Email and the right person to weigh in here.

Comment 24

[Casey Yee]

Having the error feedback as per Andrews comment 22 is what is important here.   The user still has the ability to manually re-check email which in my opinion is sufficient for V1.

Comment 25

[Dylan Oliver [:doliver]]

We haven't seen problems in this area for some time -- please renominate if this pops up again.

Comment 26

[Jim Porter (:squib)]

Yeah, I'd say this is one of those bugs which would be nice to have if someone had a little bit of free time, but otherwise it's not a big deal. Since ActiveSync doesn't keep connections open*, retrying happens at a different layer, and shouldn't cause problems. Fixing this might make things a little more efficient/friendly though.

* If we enable push mail, this might become more important, since that involves long-lived connections.

Comment 27

[Jim Porter (:squib)]

I'm not working on this, so resetting the assignee.

Comment 28

[Mike Hoye [:mhoye]]

This bug is looking for a mentor. Casey?

Comment 29

[Jim Porter (:squib)]

I'm probably the most obvious mentor for this, but I don't know if we even need this anymore. We could possibly just close it (but it'd be best to ask Andrew when he gets back from vacation).

Comment 30

[Casey Yee]

redirecting to juwei since she is the ux owner for this.

Comment 31

[Juwei Huang[:juwei]]

Hi Andrew, is this bug resolved or we need an alternate ux solution for it?

Comment 32

[Andrew Sutherland [:asuth] (he/him)]

I'm going to just resolve this as WFM since I think we ended up implementing all the ActiveSync fixes we thought we needed already on other bugs.  Things like us not telling the user we're offline are not ActiveSync specific and are handled by existing UI bugs.

The only thing we could do that we didn't do was do more exponential backoff stuff, but as noted in many places, it's not clear what win that gets us.  IMAP has (or at least we're more aware of) many transient failure modes that we understand and where our backoff is appropriate.  And we've seen servers generate those failures in our experience.  While ActiveSync has some state indication like that, we generally have not seen it used on Microsoft-authored implementations, suggesting it's not super-needed.  And on the other implementations, well, they tend to be bad and they probably have learned not to generate errors clients don't expect.

We'll file new bugs for any enhancements we can think of for ActiveSync or just address them on bugs as the need arises.