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

exceptions: elegantly handle known errors #2982

Closed
oliver-sanders opened this issue Mar 8, 2019 · 3 comments · Fixed by #2995
Closed

exceptions: elegantly handle known errors #2982

oliver-sanders opened this issue Mar 8, 2019 · 3 comments · Fixed by #2995
Assignees
Milestone

Comments

@oliver-sanders
Copy link
Member

oliver-sanders commented Mar 8, 2019

The Problem

In the past we have wrapped client functionality like so:

if __name__ == "__main__":
    try:
        main()
    except Exception as exc:
        if cylc.flags.debug:
            raise
        sys.exit(str(exc))

Unfortunately this often results in cryptic error messages which totally miss the point, often like this:

TypeError: unsupported operand type(s) for +: 'NoneType' and 'int'

The alternative would be to let the traceback reach the user:

if __name__ == "__main__":
    main()

But this results in massive stack traces which obfuscate the actual issue, take for example this simple case of a user trying to call a remote command by the wrong name:

$ cylc client generic ping -n
Traceback (most recent call last):
  File "/cylc/bin/cylc-client", line 63, in <module>
    main()
  File "/cylc/bin/cylc-client", line 58, in main
    pclient(func, kwargs, timeout=options.comms_timeout), indent=4
  File "/cylc/lib/cylc/network/client.py", line 194, in serial_request
    self.async_request(command, args, timeout))
  File "/python3.7/asyncio/runners.py", line 43, in run
    return loop.run_until_complete(main)
  File "/python3.7/asyncio/base_events.py", line 584, in run_until_complete
    return future.result()
  File "/cylc/lib/cylc/network/client.py", line 190, in async_request
    raise ClientError(response['error'])
cylc.network.client.ClientError: Request returned error: No method by the name "ping"

All the client actually need to see is No method by the name "ping"!

To add to this issue in Python3 the old pattern of catching a nasty error and raise a nicer one can backfire as Python now prints the full stack trace for both. This can cause traceback to get very long very quickly:

>>> try:
...     1/0
... except ZeroDivisionError:
...     raise MyException('Some nice error to handle this.')
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
MyException: Some nice error to handle this.

See also:

The Solution?

Put in place a system where we can provide nice one-line messages for "expected" errors but display the full traceback for "unexpected".

This could be achieved by raising "expected" errors with a special exception and sticking decorators on all the main functions called by the client.

def interactive(function):
    if is_terminal():
        try:
             function()
        except CylcError as exc:
             sys.exit(exc)
    else:
        # raise full traceback in non-interactive environments?
        function()

This should be a pretty small job to put in place.

Ideas?

  • Ideas for nice ways of doing this?
  • Any good examples from other Python projects?
@oliver-sanders oliver-sanders added this to the cylc-8.0.0 milestone Mar 8, 2019
@oliver-sanders oliver-sanders mentioned this issue Mar 8, 2019
5 tasks
@matthewrmshin
Copy link
Contributor

Sounds like a good plan.

@hjoliver
Copy link
Member

Yes, good plan!

@oliver-sanders
Copy link
Member Author

Note: include change from #2974

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

Successfully merging a pull request may close this issue.

3 participants