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

shouldn't showing stack traces (a la --debug) be the default behaviour? #220

Closed
aspiers opened this issue Jan 29, 2019 · 6 comments
Closed

Comments

@aspiers
Copy link

aspiers commented Jan 29, 2019

Issue description

As mentioned in #219, it is possible to cause stestr to crash with an error min() arg is an empty sequence, but with no stacktrace. Adding --debug gives the stacktrace, but this begs the question - why are stack traces being hidden by default? It gives the false impression that a crash such as this one was the fault of the user, but as a result the error message will focus on something which went wrong internally, and as a result will generally appear cryptic to the user.

Steps to reproduce the problem

Exploit a bug such as the one mentioned in #219 in order to cause stestr to crash.

Expected behavior

The stacktrace is shown, making it obvious that this was a crash not a usage error caused by the end user.

Actual behavior

Only the error message is shown, with no indication the user hit a bug in stestr.

Specifications like the version of the project, operating system, or hardware

Same as in #219.

@mtreinish
Copy link
Owner

I agree with this, and always disliked the error message only without a stack trace by default. But, this comes from the default behavior in cliff and I do not know if there is a way to change how cliff behaves on this. If there is I'm definitely in favor of making sure we always print stack traces on unhandled exceptions

@aspiers
Copy link
Author

aspiers commented Jan 30, 2019

@mtreinish commented on 29 Jan 2019, 17:43 GMT:

I agree with this, and always disliked the error message only without a stack trace by default. But, this comes from the default behavior in cliff and I do not know if there is a way to change how cliff behaves on this. If there is I'm definitely in favor of making sure we always print stack traces on unhandled exceptions

Well, as you well know cliff is Open Source maintained by excellent folks ;-) So I think we can confidently say there is a way to change how cliff behaves on this, even if that way requires some code changes :-)

Based on a quick glance it appears to have been added here:

openstack/cliff@276e8a4

so it's time to chat with @dhellmann - I'm sure he had a good reason for adding it :-) Maybe we can catch him in IRC.

@dhellmann
Copy link

Stack traces are a terrible way to report an error to a normal application user, so I don't think we want to change the default behavior in cliff. If you wanted to change the default for stestr, then we could update cliff to make it possible to override the default. It may already be possible to do that by modifying self.options.debug in the initialize_app() method, so give that a try first.

mtreinish added a commit that referenced this issue Jan 30, 2019
By default cliff suppresses stack traces on unhandled exceptions in order
to keep the output cleaner for users. But in practice this has caused more
confusion for stestr users since often the error messages aren't really
descriptive enough to indicate what's going wrong (or that it's even a
bug in stestr). This is often because exception messages are written
assuming the context of the stack trace for debugging, not to be user
facing messages. Cliff offers the --debug flag to enable printing the
full stack traces. This commit turns that flag on by default when stestr
initializes the cliff App object to ensure that it always prints the
full stack trace on an unhandled exception.

Fixes #220
@aspiers
Copy link
Author

aspiers commented Jan 30, 2019

@dhellmann commented on January 30, 2019 3:52 PM:

Stack traces are a terrible way to report an error to a normal application user

Yes, but as stated in IRC, I think a cryptic one-liner like min() arg is an empty sequence is even worse, because it leaves the user wondering whether they are supposed to understand that, and whether it was their fault. With a stack trace it's more obvious. Furthermore, if it's hard to reproduce, then re-running with --debug in order to capture essential debug to include in a bug report is not necessarily an option.

So here's an idea for how we could do even better:

  1. Keep --debug behaviour as it is.

  2. When --debug is not specified and an exception is caught, write the full error and stack trace to a file, and then output a much more user-friendly error to the terminal making it clear that this was a crash, e.g.

    BUG: min() arg is an empty sequence
    
    Unfortunately $PROGRAM unexpectedly crashed.  Please consider searching
    the issue tracker at
    
        $URL
    
    to see if this is a known issue, and if it hasn't already been reported,
    please report it, including the stack trace which has been saved to:
    
        $TRACE_FILE
    

Of course since this is handled in cliff, we'd want a way for application writers to customise the error message. @dhellmann Would you consider a patch to implement this?

@aspiers
Copy link
Author

aspiers commented Jan 30, 2019

Also, specifying the URL of an issue tracker could be optional for the application author, in which case the default error template could automatically adjust depending on whether it's provided or not.

@aspiers
Copy link
Author

aspiers commented Jan 30, 2019

I actually did something similar in another program of mine a long time ago, where there was even the possibility to guess that the crash was associated with known bugs:

https://github.com/aspiers/ly2video/blob/17327d4b021beb71a7e00d3995b79257eef330a0/ly2video/utils.py#L56

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

No branches or pull requests

3 participants