throwing non-errors (specifically strings)

[jonathanong]

is there a rule to make sure people don't throw strings?

[nzakas]

We do not. For future reference, our rules are all documented: http://eslint.org/docs/rules/

[jonathanong]

Well this is a feature request then :)

[nzakas]

Please take a moment to read over what new rule proposals should include: http://eslint.org/docs/developer-guide/contributing.html#new-rules

[doberkofler]

I would very much like to see this as a build-in rule.

[nzakas]

Please put together a proposal for the rule per the link.

[doberkofler]

I must start by saying that I have very strict guidelines on what can be thrown, so this might be far to restrictive for others.

I would want this rule to basically enforce only throwing errors in the form throw new Error() where Error should be one of an arbitrary list of constructors that can be specified in the rule definition.

The rule should be disabled by default, could be part of the Best Practices and could be specified with something like this:

"throw-style": [1, { "force-new": true, "constructors": ["Error", "TypeError", "CustomError"]}]

[gcochard]

👍

[nzakas]

There's a lot more details to hash out here. We can easily flag patterns like:

throw "foo";

We can also notice

throw new Error();

What we can't easily do is notice

var err = new Error();
throw err;

I don't know that it's worth trying to limit the constructor being used given that.

[doberkofler]

As I mentioned, I would be fine to only allow throw new Error() as I have very strict rules as I almost never use any other form of exception. Clearly for others this might be to restrictive.
Looking at the code I see around throwing a variable does not seem to very common.

[nzakas]

The larger question around this rule is this: should it disallow specific patterns or allow specific patterns?

[doberkofler]

I would only allow one pattern

[ilyavolodin]

If it will allow only one pattern (throw new Error), you will have issues with code like this:

try {
   ....
} catch (e) {
   ....
   throw e;
}

This feels specific enough to your coding style to be a custom rule, instead of going into main repo.

[nzakas]

Only allowing one pattern is far too limiting. I'm leaning more towards looking for specific bad examples and warning them.

[doberkofler]

I agree with @ilyavolodin that the pattern

try {
   ....
} catch (e) {
   ....
   throw e;
}

is so common that it should also be allowed, but I'm not so sure if there are other reasonably common code pattern for throwing exceptions.

Am I guessing right, that supporting the above pattern would be be possible in eslint by matching the argument e with the object e that will be thrown ?

[nzakas]

I'm not in favor of ESLint detecting such a pattern. We know from experience that there are frequently an unknown number of exception cases when we try to detect things, which is why we opt for detecting small, known cases rather than trying to account for everything.

I'm getting closer to being convinced that we should simply detect throw followed by a literal and then warn that an error should be used.

[xjamundx]

Oh ha I already built this a few weeks ago. I should have mentioned it :) Here's how mine would ideally work:

• you can always re-throw params
• basic variable watching (within the file) to ensure that the variable you're throwing was declared as an error or a param
• support for new CustomError or whatever as well (we extend errors all the time here), either through a whitelist or just basically assume new Thing is an error
• Would also strongly consider warning against cb("ljkasjklds") or done("alskjjlkds").

(may revisit this with more thoughts, but +1000 I love this rule)