Community
Participate
Working Groups
The class org.eclipse.jface.util.SafeRunnable opens an error dialog if the runnable it contains throws an exception. There are cases (e.g bug 46960) where an exception in this runnable can happen frequently. I got 137 error dialogs when I was reproducing this problem. Would logging the error be sufficient in this case? Otherwise, some mechanism to avoid poping up multiple dialogs at the same time should be employed.
You can suppress this by using ignoreErrors=true in your SafeRunnable.
The "you" in this case is StructuredViewer/TreeViewer. The SafeRunnable is used by the TreeViewer during label updating (doUpdateItem) and the StructuredViewer (refreshItem). Sorry for not making that clearer in the original description. When an error occurs for a single item, an error dialog is presented. If multiple items have errors, a dialog is presented for each. In the rare case where this happens, you can end up with a lot of dialogs. I don't know if logging is the best alternative since the user would like to know that an error occurred but a single dialog would be sufficient. Would it be possible to accumulate the exceptiuons and prompt the user once. If not, would some other means of error notification be possible. Closing 137 error dialogs takes a while.
This is a general problem with SafeRunnables. We have many of them in parts which open dialogs (AbstractPartSelectionTracker, StructuredViewer, etc.). The other option is to write to the log only but this give no user feedback. If we are going to change how this works then we are changing this everywhere. What we might consider (post 3.0) is some sort of error collecting dialog like we currently have for jobs. I am going to mark this asan enhancement and consider it post 3.0 as it would represent a fairly far reaching new feature and we want to minimize functionality changes.
Reopening
With the change in bug 58845 to not remove failing listeners, this situation could be made much worse. I've decided to go ahead and make the default be ignoreErrors=true, so we won't get error dialogs for free, but apps can still reverse this decision if needed. Core will still log any error in Platform.run.
Fixed.
We had to back out of this change for 3.0. See bug 66733 for details. SafeRunnable.ignoreErrors is false by default. Should find some way of routing this to an app-level event handler (something akin to WorkbenchAdvisor.eventLoopExeption).
Not for 3.1. Reassigning to Stefan since he has recently become passionate about this issue <g>.
This is not only annoying, it's incorrect. Opening a dialog is part of a method's contract, and dialogs are not permitted in most places where SafeRunnable is used.
*** Bug 87721 has been marked as a duplicate of this bug. ***
See also bug 87721 comment #4: "With the recent changes for bug 49497, the app could configure a different safe runnable runner using SafeRunnable.setRunner(ISafeRunnableRunner). See also JFaceUtil in the workbench." Susan, is bug 103295 another duplicate of this?
(In reply to comment #11) > See also bug 87721 comment #4: > "With the recent changes for bug 49497, the app could configure a different > safe > runnable runner using SafeRunnable.setRunner(ISafeRunnableRunner). > See also JFaceUtil in the workbench." Note that by doing this, you can only replace the error dialog with custom behaviour for those cases where SafeRunnable.run() or SafeRunnable.getRunner().run() is used. If subclasses of SafeRunnable are run using other means, e.g. through equinox common's SafeRunner.run() or Platform.run(), the error dialog cannot be replaced. We should fix this for 3.3.
>Susan, is bug 103295 another duplicate of this? Not exactly. The reporter of that bug never responded to the final question, but it would seem that the desire was to be able to set up a SafeRunnable runner that would also work with InternalPlatform.run. Which I think is what you are saying also in comment #12. To me, that implies work on the runtime side, to allow a similar way for apps to set up the runnable runners. JFace can't do this since it is supposed to be independent of the runtime. I will move bug #103295 to runtime for comment. I'm not sure what is left "to-do" on this bug. Are we considering setting a different kind of SafeRunnableRunner in the IDE to avoid multiple dialogs?
(In reply to comment #13) > To me, that implies work on the runtime side, to allow a similar way for apps > to set up the runnable runners. JFace can't do this since it is supposed to be > independent of the runtime. Maybe I'm missing something, but if any runnable runner calls ISafeRunnable.handleException() - btw this is required if you implement ISafeRunnableRunner -, the default implementation in SafeRunnable will open a dialog. To fix this, we need additional API in SafeRunnable so that applications can intercept the exception and do with it what they want.
I see...I was distracted by the suggestion to fix the problem by replacing the SafeRunnableRunner. Using the runner to avoid the problem only works when SafeRunnableRunner is used. Just to make sure I understand the problem. SafeRunnable is expressly documented as an abstract implementation of ISafeRunnable that opens a message dialog on error. Implementors have to subclass this to provide the run() behavior, and presumably can implement handleException however they wish. But there are so many built-in uses of SafeRunnable in JFace itself that we need to provide a hook into the handleException behavior that would be set at the SafeRunnable level, ie an exception handler that can be assigned statically to SafeRunnable, and if none is assigned we can revert to the default (message dialog) behavior. Does that sound about right?
Yes this sounds right to me. Ideally, for RCP applications, there should be just one place e.g. in the WorkbenchAdvisor where they define what happens with unexpected errors. Remember that the IDE is just another RCP application... This could be another aspect of JFace configured by the Workbench by calling API on Policy.java.
Susan, should I just take this bug?
sure, it sounds like you have a good grasp from the RCP point of view. I was thinking the new API to set the exception handler would be called in JFaceUtil.initializeJFace(), where the other Policy calls are made. Note there are calls to SafeRunnable as well (setRunner), so I was leaning toward making the exception handler setter static API on the SafeRunnable class. The interface itself could be named IExceptionHandler, or perhaps more specifically ISafeRunnableExceptionHandler. That's where I was headed...
SafeRunnable now opens a non-modal dialog, collecting errors if necessary. This avoids the multiple error dialog case, and will not spin the event loop at inappropriate times. See bug 178906. *** This bug has been marked as a duplicate of bug 178906 ***
