Closed Bug 311266 Opened 17 years ago Closed 17 years ago

Fix for bug 306961 was too aggressive, causes partial update to keep failing in some cases where a full update should be downloaded

Categories

(Toolkit :: Application Update, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: darin.moz, Assigned: darin.moz)

Details

(Keywords: fixed1.8, regression)

Attachments

(1 file)

Fix for bug 306961 was too aggressive.  I chose to treat any I/O error as a
temporary error, but updater.cpp generates an I/O error if, for example, the old
file being patched doesn't exist.  In that case, the update service should give
up trying to apply the partial patch and failover to downloading the complete
archive.  The fix, I think, is to adjust the error codes generated by
updater.cpp when it fails to apply a patch.  It might make sense to distinguish
write errors from read errors to solve this bug.
Keywords: regression
Blocks: 311165
Darin/Chase and I think we need to block on this bug.
Flags: blocking1.8b5+
Attached patch v1 patchSplinter Review
Attachment #198645 - Flags: review?(benjamin)
When we hit a failure to apply a partial update, there are two paths we may
take.  First, we may decide that the failure is intermittent (see bug 306961),
in which case we want to leave the patch as is to be applied later.  Otherwise,
we decide to skip the partial patch and proceed to download a complete patch.

I've tested the following error cases on Windows and Linux:

1) Attempt partial patch, but one of the source files cannot be modified.
2) Attempt partial patch, but one of the source files does not exist.
3) Attempt partial patch, but one of the source directories does not exist. 
(variant on #2)
4) Attempt partial patch, but one of the source files does not contain the data
expected by the patch.

With this patch, testcase #1 results in Firefox prompting the user to "correct
the problem and retry."  In the other testcases, Firefox reverts to downloading
the complete patch.

Without this patch, testcases #2 and #3 are not handled properly.

The patch for this bug looks large, but it is actually pretty straightforward. 
I'm basically just replacing IO_ERROR with READ_ERROR or WRITE_ERROR to better
describe the specific failure.  Then, I only invoke the "retry" prompt when we
encounter a WRITE_ERROR.  As part of this patch I also got rid of the BSP_ error
codes in favor of the ones defined in errors.h.
Comment on attachment 198645 [details] [diff] [review]
v1 patch

we're taking this now but we still need benjamin to sign off on the patch.
Please review this ASAP Benjamin.

If he finds any issues we'll have to consider another respin again, but we'd
rather get builds we can start testing with instead of blocking the respin
until we can find benjamin.
Attachment #198645 - Flags: approval1.8b5+
Attachment #198645 - Flags: superreview?(benjamin)
fixed-on-trunk, fixed1.8
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
No longer blocks: 311165
Attachment #198645 - Flags: superreview?(benjamin) → superreview+
Summary: Fix for bug 306961 was too aggressive → Fix for bug 306961 was too aggressive, causes partial update to keep failing in some cases where a full update should be downloaded
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.