Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subcommand method ParameterException is hidden by InvocationTargetException #554

Closed
SysLord opened this issue Nov 22, 2018 · 4 comments
Closed
Milestone

Comments

@SysLord
Copy link
Contributor

SysLord commented Nov 22, 2018

executionResult.add(((Method) command).invoke(parsed.getCommandSpec().parent().userObject(), parsed.getCommandSpec().argValues()));

Actual behaviour: Throwing a ParameterException from a command method is wrapped in InvocationTargetException and is not catched and handled as ParameterException. No help output.
Expected behaviour: ParameterExceptions get handled like in Callable or Runnable methods and print help.

@remkop
Copy link
Owner

remkop commented Nov 22, 2018

Can you please provide a way to reproduce the problem? Ideally some source code for the command and user input that triggers the problem.

@remkop
Copy link
Owner

remkop commented Nov 26, 2018

I was able to reproduce the issue with the following test:

class Test538 {
    @Rule
    public final SystemErrRule systemErrRule = new SystemErrRule().enableLog().muteForSuccessfulTests();

    @Rule
    public final SystemOutRule systemOutRule = new SystemOutRule().enableLog().muteForSuccessfulTests();

    /** Test for https://github.com/remkop/picocli/issues/554 */
    @Command(name = "maincommand")
    class MainCommand implements Runnable {
        @Spec CommandSpec spec;

        public void run() { throw new UnsupportedOperationException("must specify a subcommand"); }

        @Command
        public void subcommand(@Option(names = "-x") String x) {
            System.out.println("x=" + x);
        }

        @Command
        public void explicit(@Option(names = "-v") boolean v) {
            CommandLine commandLine = spec.subcommands().get("explicit");
            throw new CommandLine.ParameterException(commandLine, "Validation failed");
        }
    }

    @Test
    public void testSubcommandMethodExceptionHandling() {
        String expected = String.format("" +
                "Unknown option: -y%n" +
                "Usage: maincommand subcommand [-x=<arg0>]%n" +
                "  -x=<arg0>%n");

        CommandLine.run(new MainCommand(), "subcommand", "-y");
        assertEquals(expected, this.systemErrRule.getLog());
        assertEquals("", this.systemOutRule.getLog());
    }

    @Test
    public void testSubcommandMethodThrowingParameterException() {
        String expected = String.format("" +
                "Validation failed%n" +
                "Usage: maincommand explicit [-v]%n" +
                "  -v%n");

        CommandLine.run(new MainCommand(), "explicit", "-v");
        assertEquals(expected, this.systemErrRule.getLog());
        assertEquals("", this.systemOutRule.getLog());
    }
}

Note to self: the patch below fixes the problem.

Index: src/main/java/picocli/CommandLine.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/java/picocli/CommandLine.java	(revision )
+++ src/main/java/picocli/CommandLine.java	(revision )
@@ -1081,12 +1081,17 @@
                     }
                     throw new UnsupportedOperationException("Invoking non-static method without default constructor not implemented");
                 }
-            } catch (ParameterException ex) {
-                throw ex;
-            } catch (ExecutionException ex) {
-                throw ex;
+            } catch (InvocationTargetException ex) {
+                Throwable t = ex.getTargetException();
+                if (t instanceof ParameterException) {
+                    throw (ParameterException) t;
+                } else if (t instanceof ExecutionException) {
+                    throw (ExecutionException) t;
+                } else {
+                    throw new ExecutionException(parsed, "Error while calling command (" + command + "): " + t, t);
+                }
             } catch (Exception ex) {
-                throw new ExecutionException(parsed, "Error while calling command (" + command + "): " + ex, ex);
+                throw new ExecutionException(parsed, "Unhandled error while calling command (" + command + "): " + ex, ex);
             }
         }
         throw new ExecutionException(parsed, "Parsed command (" + command + ") is not Method, Runnable or Callable");
@@ -9926,8 +9931,8 @@
             super(msg);
             this.commandLine = Assert.notNull(commandLine, "commandLine");
         }
-        public ExecutionException(CommandLine commandLine, String msg, Exception ex) {
-            super(msg, ex);
+        public ExecutionException(CommandLine commandLine, String msg, Throwable t) {
+            super(msg, t);
             this.commandLine = Assert.notNull(commandLine, "commandLine");
         }
         /** Returns the {@code CommandLine} object for the (sub)command that could not be invoked.

I will apply this fix as soon as I can.

@remkop remkop added this to the 3.8.1 milestone Nov 26, 2018
@SysLord
Copy link
Contributor Author

SysLord commented Nov 26, 2018

Exactly. Thank you for looking into this. I just went looking into providing the required information, but you were faster.

@remkop remkop closed this as completed in 46f176d Nov 28, 2018
@remkop
Copy link
Owner

remkop commented Nov 28, 2018

Fixed in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants