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

Hide stack traces unless debug mode is enabled. #135

Closed
danielhollas opened this issue Jun 24, 2022 · 4 comments · Fixed by #148
Closed

Hide stack traces unless debug mode is enabled. #135

danielhollas opened this issue Jun 24, 2022 · 4 comments · Fixed by #148
Assignees

Comments

@danielhollas
Copy link
Contributor

danielhollas commented Jun 24, 2022

I always find it jarring when as a user a python CLI tool spews a full stack trace into my terminal. More importantly, I always have to squint to find tha actual error.

I wonder whether it wouldn't be a more user friendly to disable stack traces unless a debug mode is enabled? Something.like below perhaps?
https://stackoverflow.com/a/27674608

I am asking both as a user of this specific package, but also in general as a newbie Python dev about what's the best practice here.

@csadorf
Copy link
Member

csadorf commented Jun 27, 2022

I agree, there should ideally never be a traceback visible to the user which is why all expected exceptions should be caught and handled in https://github.com/aiidalab/aiidalab-launch/blob/main/aiidalab_launch/__main__.py .

However, it is of course possible that something "falls through the cracks", and setting the tracebacklimit to 0 could be a viable solution to address that. But I assume that the error message would potentially be even more cryptic, the traceback dump at least indicates that something went "horribly wrong". Using a custom sys.excepthook exception handler might be a better option, where we can at least show a message that explains that the program crashed with an unexpected error.

Maybe it would be worthwhile to find an example of such an implementation in a "popular" CLI application that we could compare against?

Did you encounter a traceback with the package or is this just a general question?

@danielhollas
Copy link
Contributor Author

Here's an example of what happens when you provide an non-existent profile name.

$ aiidalab-launch profiles edit invalid
Traceback (most recent call last):
  File "/home/bl22441/.local/bin/aiidalab-launch", line 8, in <module>
    sys.exit(cli())
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/decorators.py", line 84, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/aiidalab_launch/__main__.py", line 245, in edit_profile
    current_profile = app_state.config.get_profile(profile)
  File "/home/bl22441/.local/pipx/venvs/aiidalab_launch/lib/python3.8/site-packages/aiidalab_launch/config.py", line 65, in get_profile
    raise ValueError(f"Did not find profile with name '{name}'.")
ValueError: Did not find profile with name 'invalid'.

It would be great if only the last line was printed, or if we need more context to limit the traceback depth to 1 for example.

Also all the additional validations that we added for extra mounts throw ValueError which I think is not handled in __main__.py

@csadorf
Copy link
Member

csadorf commented Jul 19, 2022

@danielhollas Would you be open to trying to implement this?

@danielhollas
Copy link
Contributor Author

@csadorf sure, I can give it a shot, feel free to assign this to me. Although it's a low priority for now so if somebody else wants to go ahead feel free.

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.

2 participants