Traceback is not useful when a command is missing arguments

[bennuttall]

When running a command without its required arguments, the following traceback is shown:

$ python api.py create
Traceback (most recent call last):
  File "/Users/nuttab01/Projects/bbc/newslabs-highlights/cli/api.py", line 111, in <module>
    app.main()
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 4203, in main
    sys.exit(self.process(args))
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 4195, in process
    result = self.execute(commands)
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 4161, in execute
    result = command.execute()
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 2296, in execute
    return self.callable(*self.args, **self.kwargs)
TypeError: create() missing 2 required positional arguments: 'title' and 'in_time'

My thoughts are that this is a perfectly normal expected error for the user to make, and that while the exception message provides useful context, the traceback is just noise. At this point they just need to be reminded of the usage for this command, and informed of which arguments were missing.

It feels like this should just be handled and wrapped up in a simple error message. Perhaps the exception message could be reformatted slightly to be more about the parser's expectations than the Python function's expectations.

[larryhastings]

Yeah, Appeal should catch that error. It does report missing parameters in other contexts but I guess I missed one. Please provide a sample script that produces this specific error and I'll take a look.

I've been on the fence for a while about how far Appeal should go in catching errors. Though I swing back and forth, most of the time I think Appeal should catch and present friendly errors for everything up until the final "command" call--internally what I call the analyze, parse, and convert steps.

The real question: if you use your own class as a "converter", and that throws an exception, should the user see it, or should Appeal catch it? It would have to catch Exception, and it wouldn't really know what happened.

[bennuttall]

The simple example from the readme:

import appeal

app = appeal.Appeal()

@app.command()
def hello(name):
    print(f"Hello, {name}!")

app.main()

When run with no arguments:

$ python cli.py hello     
Traceback (most recent call last):
  File "/Users/nuttab01/Projects/bbc/newslabs-highlights/cli/cli.py", line 9, in <module>
    app.main()
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 4203, in main
    sys.exit(self.process(args))
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 4195, in process
    result = self.execute(commands)
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 4161, in execute
    result = command.execute()
  File "/Users/nuttab01/.virtualenvs/highlights/lib/python3.9/site-packages/appeal/__init__.py", line 2296, in execute
    return self.callable(*self.args, **self.kwargs)
TypeError: hello() missing 1 required positional argument: 'name'

I've been on the fence for a while about how far Appeal should go in catching errors. Though I swing back and forth, most of the time I think Appeal should catch and present friendly errors for everything up until the final "command" call--internally what I call the analyze, parse, and convert steps.

Agreed

The real question: if you use your own class as a "converter", and that throws an exception, should the user see it, or should Appeal catch it? It would have to catch Exception, and it wouldn't really know what happened.

I think it's reasonable for this to show the traceback in this case. If you wanted to, you could catch it, show a custom message (a bit like pip does now when a compilation fails: note: This error originates from a subprocess, and is likely not a problem with pip.) and then raise it with a chained exception.

[larryhastings]

Gotcha. I'll add a test for that.

That said, all the code examples in the README do get tested--automatically, by tests/run_tests.py. It parses the README, pulls out the test code, and tests it to make sure that it works.

What it mostly doesn't do is test it to see what it does when it fails. I should add more tests in that vein. Certainly I will for this one.

I shudder to think what Appeal's coverage score is...

[larryhastings]

Fixed this while on a plane the other day. See the commit message for details. But in my defense: this was a bugfix, not a missing feature!

[bennuttall]

Great, thanks Larry!