Closed Bug 1009422 Opened 10 years ago Closed 10 years ago

[email/Activesync/hotmail] Apparent change in server behaviour resulting in regression in downloadBodies due to missing body payload; results in operation hang, breaking all operations in the folder

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S5 (4july)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: asuth, Assigned: squib)

References

Details

(Whiteboard: [cert])

Attachments

(1 file)

I've now seen a v1.3t (bug 1001742) and a v1.4 regression (in a few different bugs that are all a result of the mutex issue: bug 1009296, bug 1009301, bug 1009309) reported on body fetching for ActiveSync.  Signature is slightly different between v1.3t and v1.4 because of changes in our ActiveSync implementation in terms of catching the exception.  Also, regrettably the error message reported in v1.4 appears to be less useful because of that.

This may just be an awkward outgrowth of bug 825538 (which we should now absolutely fix to avoid that complication) in conjunction with bad error handling on our part.  Or there may be a fundamental change in body-processing for HTML bodies on hotmail, but I had real trouble duplicating the problem with a source message on bug 1001742.

This and any related bugs may need to end up v1.4 blockers or approved for uplift since it's hard for QA or anyone to differentiate this from other bugs without consulting the logcat.

Currently assigning to myself for more investigation.
Note to self: The v1.4 reproduction cases involve moves.  This could be an edge-case relating to server-id's becoming out-of-date when moved between folders and our failure to update them/etc.  I believe we had some speculative logic that was guessing that ActiveSync might persistently assign message-id's.  Especially if that used to be true but is no longer true, this could explain what we're seeing.  Or we just always broke but we never got that much use of ActiveSync.
Blocks: 1008046
Requesting 1.3? since this is believed to be the root cause of that current 1.3+ blocker bug.

I'm going more seriously deep-PTO now, so Jim if you've got the cycles to lend to help wrap this call in a guard (assuming my belief is right that we're just asking for a no-longer-valid handle) and/or do something smarter and add a test, that'd be awesome.
Assignee: bugmail → nobody
Status: ASSIGNED → NEW
blocking-b2g: --- → 1.3?
Flags: needinfo?(squibblyflabbetydoo)
I've forgotten most of ActiveSync, but I'll take a look at this at some point and see if I come up with anything useful.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Flags: needinfo?(squibblyflabbetydoo)
blocking-b2g: 1.3? → 1.3+
Whiteboard: [cert]
Jim,

Andrew is out till end of June and since you have worked on activesync before, we need your help here to fix this bug. Please help!

Thanks a lot!

Hema
Flags: needinfo?(squibblyflabbetydoo)
It's not actually clear to me what's happening. I mean, I can be paranoid and write something to gracefully handle errors, but it'll be hard to test when I'm not sure what the error *is* exactly. It seems Andrew has had a hard time reproducing this too, so I doubt he'll be much help here either.
Flags: needinfo?(squibblyflabbetydoo)
Attached file Speculative fix
I'm really just guessing here. This is the only place I could find something that might stall us out: when we try to get the bodyInfo from the local FolderStorage, it can return null, in which case we'll throw an exception and never realize we finished the downloadBodies op.
Attachment #8444732 - Flags: review?(bugmail)
Actually, I guess there could be something really bad wrong in parsing the WBXML response, so let's be super-extra paranoid here.

I think the long-term solution is actually to switch GELAM (or at least GELAM's ActiveSync impl) to use Promises, which would handle exceptions for us.
Comment on attachment 8444732 [details] [review]
Speculative fix

It'd be great to have a test where we imitate what a server says if we ask it to download body reps for a message that no longer exists, but this seems like a safe improvement.  Thank you!
Attachment #8444732 - Flags: review?(bugmail) → review+
Target Milestone: --- → 2.0 S5 (4july)
Jim, is this one ready to land? We'll also need to request 1.3 approval for the patch after it lands on master.
Flags: needinfo?(squibblyflabbetydoo)
I was trying to write tests for this, but wasn't having much luck, since I couldn't figure out what the real-life servers were doing. I'll land this shortly.
Flags: needinfo?(squibblyflabbetydoo)
This is a 1.3+, it needs approval to land in 1.3, could you please ask for it?
Flags: needinfo?(squibblyflabbetydoo)
Oh FML, this got mixed in with the bugs that already had approval.
Comment on attachment 8444732 [details] [review]
Speculative fix

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 #): Possibly a change in how Hotmail's servers work
[User impact] if declined: ActiveSync email can get locked up, requiring a restart to unstick it
[Testing completed]: Unit tests to verify nothing broke (I couldn't reproduce the original issue, so the fix is speculative)
[Risk to taking this patch] (and alternatives if risky): Very low risk; I just added a couple of extra catches for potential errors.
[String changes made]: None
Attachment #8444732 - Flags: approval-gaia-v2.0?
Attachment #8444732 - Flags: approval-gaia-v1.4?
Attachment #8444732 - Flags: approval-gaia-v1.3?
Flags: needinfo?(squibblyflabbetydoo)
Er, I didn't see comment 17 and 18, but hopefully what I just posted is fine anyway...
Comment on attachment 8444732 [details] [review]
Speculative fix

We still need the v1.3 made official, yes.
Attachment #8444732 - Flags: approval-gaia-v2.0?
Attachment #8444732 - Flags: approval-gaia-v1.4?
Comment on attachment 8444732 [details] [review]
Speculative fix

Approval should have happened before the landing, just cleaning this up and re-reading the approval request comment a=1.3+ holds here.
Attachment #8444732 - Flags: approval-gaia-v1.3? → approval-gaia-v1.3+
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: