Bugzilla – Full Text Bug Listing
| Summary: | Failed download of flat bundles or features is not retried | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Pascal Rapicault <pascal> | ||||||||
| Component: | p2 | Assignee: | Pascal Rapicault <pascal> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | major | ||||||||||
| Priority: | P3 | CC: | jeffmcaffer, john.arthorne, kim.moir, philippe_mulet, simon_kaegi, tjwatson | ||||||||
| Version: | 3.4 | Flags: | pascal:
review?
(tjwatson) jeffmcaffer: review+ john.arthorne: review+ pascal: review? (Mike_Wilson) pascal: review? (dj.houghton) |
||||||||
| Target Milestone: | 3.4 RC4 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Pascal Rapicault
Created attachment 104067 [details]
Patch ensuring that retry errors are not being consumed
The patch consists in checking if the result of the download is a retry operation, and even in case of other failures propagate the retry error code.
The patch is safe since it consists in adding more checks.
Created attachment 104071 [details]
Patch to close stream created when the source is a folder
This patch fixes another problem that I encountered when trying to reproduce this one. The core problem is that when we are downloading bundles from a repo where the artifact is stored as jar (e.g. add an eclipse install as a repo), the processing steps are not being closed.
This patch simply calls the "reportStatus" method that causes the processing steps to be closed.
Created attachment 104072 [details]
Patch to the generator
Now here is a patch for the generator since when doing this work we noticed that we missed adding the download content type to the artifact that are representing features. I think that given what happens to feature we could probably leave without it, though I think it is safer to put it in.
(Marking for RC4 because we don't have any other milestone) looks good. I do wonder if we should be returning retry code (if there are more mirrors) in the first part of downloadArtifact (from the second patch) but that should only happen in cases where the repo is local. in that case it is not clear that we have mirrors so... Can you explain more why these fixes are important for 3.4.0? How likely are these failures? Patch #2 seems unrelated to the first problem, and looks like code that has been this way for quite awhile. I understand now that the first two patches are fixing problems in our fix for bug 233214 on Friday. That fix introduced the regression of not closing the stream that patch #2 is addressing. Our testing of the fix didn't catch this because we were testing from within the IDE, but the build takes a different code path that is now failing because of this regression (bug 236173). I think we must either fix this, or revert the fix in bug 233214 that caused the regression. I have tested the code path that produces the logged errors described in bug 236173 comment 0, and verified that Pascal's patch fixes it. I'm not as convinced of the urgency of patch #3. My worry is that this will exercise different code paths that we have not had time to properly test. The consequence of a corrupt feature jar is also not as severe (patch #3 adds a processing step that verifies the downloaded artifact is a zip file). One issue with patch #1: the old code had a return statement inside a finally block. This is of course a bad coding pattern because it causes arbitrary runtime exceptions and errors from the try block to be discarded. The patch removes the return statement from the finally block. Normally I would say this is a good change, but being paranoid at this point I worry that it could cause exceptions to surface that were previously consumed by the return statement in the finally block. I would opt to leave the return statement inside the finally block at this point in the interest of minimizing changes, and enter a bug to fix it up further post 3.4. RC4 is behind us, please update the target appropriately Actually the two first patches made it into RC4. The one left can wait and could be evaluated for 3.4.1. *** Bug 236173 has been marked as a duplicate of this bug. *** I'm going to mark this fixed because we did release a fix for the issue in the title in 3.4 RC4 (and we have a blocking 3.4 defect marked as a duplicate). I have entered bug 236644 for the remaining issue which is not really related to the problem described here. |