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.
Darin/Chase and I think we need to block on this bug.
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 email@example.com
Attachment #198645 - Flags: review?(benjamin) → review+
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+
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
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
You need to log in before you can comment on or make changes to this bug.