Community
Participate
Working Groups
Created attachment 167764 [details] Rethrow ProvisioningException after logging The ProvisioningUI's method loadMetadataRepository does not rethrow the ProvisioningException that occurred when loading a repository, this leads MetadataRepositoryElement to attempt to fetch it's children (IUs) which is trigger a second attempt to contact the repository. For some problems the second attempt at retrieval results in the same error displayed multiple times.
marking for review for RC1. I'm assuming the scenario is that I'll see two repository failure messages if I select a "not found" repo in the combo? (Matthew, please give me the scenario if that's not it...)
If I add a bogus site, such as "http://example.com/foo" and select it in the combo, I get the "do you wish to edit the location" message. I say no, and that's it. Selecting other sites and coming back to the site, or closing the wizard and reopening it and selecting the site, do not cause extra notification. I understand that the code would cause the fetch to keep trying to load (and then failing again in provisioning ui), but I'd like to understand better the scenario where this produces double notification.
I'm not sure which transport errors trigger popups, but I ran into the problem using invalid credentials for a proxy. This error triggers the workbench popup which has duplicate error messages. The error is originally thrown from AbstractRepositoryManager line 684 if that helps.
Definitely this code should be revisited, but I believe this is the wrong time. Problems with the code: - the javadoc says we throw ProvisionException when in fact we don't. - one could assume that we should never get null back from this method since the signature says we throw an exception. As it is, null can be returned. Something should be done. However logging and rethrowing the exception will just introduce more duplicate logging. For example, QueryableRepositoryManager does this: try { repo = loadRepository(getRepositoryManager(), uri, sub.newChild(100)); } catch (ProvisionException e) { tracker.reportLoadFailure(uri, e); } The loadRepository message above eventually calls ProvisioningUI.loadMetadataRepository. So the exception would be reported by ProvisioningUI, and then again here. Looking at all the callers of this message, I don't think we are missing any important error reporting behavior by quietly logging. The clients are all in places where we want silent reporting: - queries initiated by viewer elements - loads initiated in background filtering jobs - etc. The idea of using this helper method is that it is being called from places where the UI assumes the "real error" has been reported and that we don't interrupt the flow with error dialogs. At least that was the idea of having this helper. What I think needs to be done here is: - clarify in the javadoc when callers should use this method vs. calling the manager. (event batching, error swallowing, etc.) Be clear that if you care about certain events or exceptions, you should be using the manager to load the repo. - if we aren't going to throw ProvisionException, take it out of the method signature - if we are going to throw it, then we shouldn't also report it to the tracker. We should let the clients do that (most of them already are). - note that there are jobs that return the exception in their status, we need to ensure that the jobs are marked with the "no prompt on error" property so that errors don't fly up in the users face, for example, when filtering in the wizard.
these bugs will be my holiday projects...
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
