It appears that NS_ERROR_DOCUMENT_NOT_CACHED is reported when network offline and we currently handle this error as a failure. We should try handling it as we handle network offline.

Robert, I was going to file a separate bug to analyze the effectiveness of this patch to keep code changes and analysis separate. Does that sound good to you?

Comment on attachment 8758333 [details] [diff] [review] Patch We should first verify that our assumptions about the networking code are correct via code inspection and attempts to reproduce so clearing review request until this is done.

Patrick, this is the bug we discussed with you yesterday. Please let us know once you've been able to look into this. Thank you!

is it possible you have an app id for your caller? That would explain it easily when we were going offline. Its the easiest thing to look at. Even if that isn't the source, I'm confident that your approach is sensible - this should not be a persistent error that would get someone stuck and integrity of the ondataavailable should not be in question (other than truncation).

Comment on attachment 8758333 [details] [diff] [review] Patch Review of attachment 8758333 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Patrick! Setting this back to r?.

Comment on attachment 8758333 [details] [diff] [review] Patch Review of attachment 8758333 [details] [diff] [review]: ----------------------------------------------------------------- In the install/update team meeting today we agreed that since we have 180 days of historical telemetry data to do the "before" analysis, we can go ahead and land this now. The "before" and "after" analysis will happen in bug 1281466. Sending this to Matt and Patrick for review since Robert is on PTO.

Comment on attachment 8758333 [details] [diff] [review] Patch Review of attachment 8758333 [details] [diff] [review]: ----------------------------------------------------------------- So I'm not comfortable slapping my r+ on code I don't really know that is also thankfully otherwise well owned and maintained. that being said, I've read the patch and its consistent with the advice I gave afaict - I think its the right thing to do.

Thanks, Patrick and Matt! Updated the commit message, carrying over r+ and f+. I will wait until Monday before landing this to avoid surprises with nightly updates over the weekend.

https://hg.mozilla.org/integration/mozilla-inbound/rev/87bbd3f58b4b153facdd1a2995105ae28cfbcd21 Bug 1272585: Start handling NS_ERROR_DOCUMENT_NOT_CACHED the same way as network offline in app update. r=mhowell

Since this only landed for 50 what do you think about uplifting it?

If you're ready to analyze the telemetry data in bug 1281466, I'd be very happy to uplift this!

Is old data removed? If not, that shouldn't prevent uplifting.

Comment on attachment 8764790 [details] [diff] [review] Patch Data is kept for 180 days, so let's uplift. Approval Request Comment [Feature/regressing bug #]: n/a [User impact if declined]: This fix has the potential to dramatically improve the adoption of new Firefox versions through app update and reduce the number of orphaned users. Telemetry analysis shows that 30% or more of our orphaned users may be impacted. [Describe test coverage new/current, TreeHerder]: App update has substantial test coverage which also covers the change here. [Risks and why]: There is a theoretical risk that users may continue a failed download that we would have aborted previously. In working with the networking team and through code inspection we don't believe that this is a realistic risk at this point. The outcome for the end user would be the same: a failed download and therefore a failed update. [String/UUID change made/needed]: none

Comment on attachment 8764790 [details] [diff] [review] Patch Review of attachment 8764790 [details] [diff] [review]: ----------------------------------------------------------------- The patch provides better handling for network error. Take it in 49 aurora.

Followup: For the first time since the dashboard was implemented this result code (12 DWNLD_ERR_DOCUMENT_NOT_CACHED) was not the largest result code on the dashboard's "Out of date, of concern client last update download code (download phase)" section and the new largest result code is 0 DWNLD_SUCCESS. This confirms that this work had a significant positive effect.

I just checked telemetry and the last version that has this error code is 48.0.2. Yay!