sindresorhus / meow

🐈 CLI app helper
MIT License
3.54k stars 151 forks source link

Only show error stack if `--verbose` flag is set #18

Open SamVerschueren opened 8 years ago

SamVerschueren commented 8 years ago

We discussed this briefly in a commit in dev-time-cli.

At the moment, when an error occurs and it is not handled by the developer, the entire error stack is shown.

devTime(cli.input[0], cli.flags).then(result => {
    console.log(time);
});
$ dev-time 0
TypeError: Expected a user
    at module.exports (/Users/sam/Projects/opensource/dev-time-cli/node_modules/dev-time/index.js:59:25)
    at Object.<anonymous> (/Users/sam/Projects/opensource/dev-time-cli/cli.js:41:1)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Function.Module.runMain (module.js:467:10)
    at startup (node.js:134:18)
    at node.js:961:3

This was before I disabled argument type parsing

When an error occurs, you got that "ugly" error string representation with the entire stacktrace. While this is very usefully, especially for debugging and reporting issues, it's not that user friendly.

proposal

Show a nice and clean error message. Something like pageres (but instead logSymbols.error).

screen shot 2015-12-09 at 10 13 19

If you want the entire stack trace, use the --verbose (or -v) flag.

Feedback and opinions more then welcome.

// @sindresorhus @kevva @Qix- @arthurvr

sindresorhus commented 8 years ago

I considered that, but realized I dread getting bug reports without any stack and then having to constantly tell the user to run it with --verbose flag and then update the issue. I think the creator is a lot more abt at deciding what should have stack or not. Just need a better way to indicate it.

SamVerschueren commented 8 years ago

but realized I dread getting bug reports without any stack and then having to constantly tell the user to run it with --verbose flag

Realized this as well when writing this issue. Maybe adding an extra note?

screen shot 2015-12-09 at 15 22 22

But probably that won't be enough for users to post issues with stacktraces.

sindresorhus commented 8 years ago

Yeah, won't help. --verbose is the perfect solution in theory if you ignore the reality of users.

I'd like to find a solution to this, though. Want to hear other options to consider too.

One is to start a convention of adding err.user = true for user errors and conditionalize it in meow.

Qix- commented 8 years ago

Agreed on all points made; --verbose is for a utopia of developers that don't currently use our software :+1:

SamVerschueren commented 8 years ago

Actually it's possible to assign undefined to err.stack

function hello() {
    const err = new Error('Hello World');
    err.stack = undefined;

    throw err;
}

hello();
//=> Error: hello world

But maybe that's kinda hacky.

sindresorhus commented 8 years ago

I know, but we still want the stack. Just not show it.

SamVerschueren commented 8 years ago

Then the only option is to add an extra flag. We just have to decide which one.

Qix- commented 8 years ago

Wishing the concealed render mode ansi escape just stuck a bunch of text into a single invisible character so that when the user copied it would copy the entire stack, but not show it.

Or that the console had a copy signal or something.

SamVerschueren commented 8 years ago

so that when the user copied it would copy the entire stack

They would probably remove it when pasting in GitHub as it was not visible in the console :).

Qix- commented 8 years ago

Potentially. Hmm.

sindresorhus commented 8 years ago

I just wish users didn't do dumb things :p

sindresorhus commented 8 years ago

I don't want noStack. It only describes what it is, not its intent.

sindresorhus commented 8 years ago
SamVerschueren commented 8 years ago

I just wish users didn't do dumb things :p

^ This :)

SamVerschueren commented 8 years ago

unicorn is fine for me :)!

We could create a unicorn-error package that uses @Qix- error-ex package

const UnicornError = require('unicorn-error');

throw new UnicornError('bar');

It behaves as a normal Error (err.name equals Error) but only has err.unicorn set to true. Benefit that it will look like a normal error in other environments but when used with meow, it looks nice and user friendly.

sindresorhus commented 8 years ago

While unicorn would be fun, we probably should be more practical. So putting on my serious hat, maybe err.user or err.human would be a good choice? Not a big fan of an error subclass. I'd rather just set a property on any error than having to subclass it.

SamVerschueren commented 8 years ago

So putting on my serious hat

Didn't knew you had one ;)

I prefer err.human

Qix- commented 8 years ago

Sindre is only truly serious about Thai food.

I almost prefer err.friendly. More analogous to other uses of the term "friendly" in computer science.

kevva commented 8 years ago

I think either err.friendly or err.human sounds good. I don't have any better alternatives at least.

Qix- commented 8 years ago

ohai @kevva :dancer:

I'm leaning more towards err.friendly.

err.human is referring to human readable but doesn't really make sense as "human" by itself. "Friendly", on the other hand, makes much more sense standing on its own.

However this is the most insane amount of bikeshedding at this point.