Community
Participate
Working Groups
Using M5, I have a case where I try to start a command handler that will fail (I removed some required bundles on purpose). I was checking an error handling code. The problem is that the first time the command is run it looks like it is successful when it failed to terminate normally. Here is the stack trace I have: org.eclipse.core.runtime.CoreException: Plug-in org.eclipse.equinox.p2.ui.sdk was unable to load class org.eclipse.equinox.internal.p2.ui.sdk.InstallNewSoftwareHandler. at org.eclipse.core.internal.registry.osgi.RegistryStrategyOSGI.throwException(RegistryStrategyOSGI.java:180) at org.eclipse.core.internal.registry.osgi.RegistryStrategyOSGI.createExecutableExtension(RegistryStrategyOSGI.java:162) at org.eclipse.core.internal.registry.ExtensionRegistry.createExecutableExtension(ExtensionRegistry.java:874) at org.eclipse.core.internal.registry.ConfigurationElement.createExecutableExtension(ConfigurationElement.java:243) at org.eclipse.core.internal.registry.ConfigurationElementHandle.createExecutableExtension(ConfigurationElementHandle.java:51) at org.eclipse.ui.internal.handlers.HandlerProxy.loadHandler(HandlerProxy.java:335) at org.eclipse.ui.internal.handlers.HandlerProxy.execute(HandlerProxy.java:274) at org.eclipse.core.commands.Command.executeWithChecks(Command.java:476) at org.eclipse.pde.api.tools.ui.internal.preferences.ApiErrorsWarningsConfigurationBlock$4.mouseDown(ApiErrorsWarningsConfigurationBlock.java:1299) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:179) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1003) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3877) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3470) at org.eclipse.jface.window.Window.runEventLoop(Window.java:825) at org.eclipse.jface.window.Window.open(Window.java:801) at org.eclipse.ui.internal.OpenPreferencesAction.run(OpenPreferencesAction.java:65) at org.eclipse.jface.action.Action.runWithEvent(Action.java:498) I think the problem comes from: public final Object execute(final ExecutionEvent event) throws ExecutionException { if (loadHandler()) { if (!isEnabled()) { MessageDialog.openInformation(Util.getShellToParentOn(), WorkbenchMessages.Information, WorkbenchMessages.PluginAction_disabledMessage); return null; } return handler.execute(event); } return null; } where null is returned is the loading of the handler failed. loadHandler is swallowing the core exception. So the best thing to do is to replace the: return null; statement with a throw new ExecutionException(..); This will make sure that the user code can properly detect an execution failure. Steps to reproduce: 1) Define a new command handler that throws a CoreException 2) Run the corresponding command with a try/catch for all potential exceptions thrown by Command.executeWithChecks(..). 3) You won't hit any of the catch blocks on the first run.
I think we shouldn't be returning null as there is no way to differentiate whether the loading of handler failed or the actual handler returned null. Should we be throwing an exception if the loadHandler fails?
This is my point. null cannot be returned if the handler failed to load. An exception is required.
Created attachment 125749 [details] Tests v01 (In reply to comment #1) > I think we shouldn't be returning null as there is no way to differentiate > whether the loading of handler failed or the actual handler returned null. > Should we be throwing an exception if the loadHandler fails? It doesn't happen that way. If the proxy is unable to load the handler, the NotHandledException is thrown. Patch with the tests is attached to show that the framework works as expected. > Steps to reproduce: > 1) Define a new command handler that throws a CoreException > 2) Run the corresponding command with a try/catch for all potential exceptions > thrown by Command.executeWithChecks(..). > 3) You won't hit any of the catch blocks on the first run. Can you verify this steps? You should be hitting the NotHandledException
Created attachment 126481 [details] Tests v02 (In reply to comment #3) > Created an attachment (id=125749) [details] > Tests v01 The problem with this test is that reading the registry tries (and fails) to load the class long before the test is executed, because the bundle org.eclipse.ui.tests is already active. The problem is only on the first try, after that it defaults to a NotHandledException. I've moved the test into the dynamic tests (thankfully didn't have to rebuild the jar :-) where it fails, no exception is thrown (because it is eaten in org.eclipse.ui.internal.handlers.HandlerProxy.loadHandler()) PW
Prakash, Can you reproduce the issue with Paul's test case ?
Created attachment 128730 [details] Tests v03 OK, that file was munged dos file checked in from a unix platform (the worst of both worlds). I've fixed it and can re-apply this patch. It includes a test that currently fails in CommandsExtensionDynamicTest PW
I was hoping to get this fixed for M6. The UI from API Tools doesn't behave well right now because of this bug. Anyway too late now for M6...
Created attachment 129415 [details] Patch v01 Patch with the tests
(In reply to comment #8) > Created an attachment (id=129415) [details] > Patch v01 When I add the changes and run, I get a failure: } catch (Exception e) { if (!(e instanceof NotHandledException)) fail("Unexpected exception while executing command", e); } junit.framework.AssertionFailedError: Unexpected exception while executing command: org.eclipse.core.commands.ExecutionException: Exception occured when loading the handler at junit.framework.Assert.fail(Assert.java:47) at org.eclipse.ui.tests.harness.util.UITestCase.fail(UITestCase.java:103) at org.eclipse.ui.tests.dynamicplugins.CommandsExtensionDynamicTest.testNonExistingHandler(CommandsExtensionDynamicTest.java:299) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at java.lang.reflect.Method.invoke(Method.java:599) at junit.framework.TestCase.runTest(TestCase.java:164) at junit.framework.TestCase.runBare(TestCase.java:130) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:120) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390) at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62) at org.eclipse.pde.internal.junit.runtime.UITestApplication$1.run(UITestApplication.java:114) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:133) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3855) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3476) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2393) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2357) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2209) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:499) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:492) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113) at org.eclipse.pde.internal.junit.runtime.UITestApplication.start(UITestApplication.java:46) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at java.lang.reflect.Method.invoke(Method.java:599) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:556) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:511) at org.eclipse.equinox.launcher.Main.run(Main.java:1284) at org.eclipse.equinox.launcher.Main.main(Main.java:1260)
Created attachment 129793 [details] fix plus tests v04 Prakash, I've checked in the plugin.xml and re-spun the patch so it doesn't include it. As I mentioned, when I run the dynamic test suites I get that failure. PW
(In reply to comment #10) > As I mentioned, when I run the dynamic test suites I get that failure. Paul, Looks like you took the testNonExistingHandler() from the older patches. The testNonExistingHandler() I had in Patch v01 will call executeCommand() twice. First time it will throw ExecutionException and from then onwards, it will always throw NotHandledException. I've commented it in the tests on reason as well. Can you try Patch v01? It clears the test
Released to HEAD >20090326 PW
Verified with I20090428-0100
