Bug: @typescript-eslint/only-throw-error False positive when throwing Error subclass

[cowwoc]

Before You File a Bug Report Please Confirm You Have Done The Following...

• [X] I have tried restarting my IDE and the issue persists.
• [X] I have updated to the latest version of the packages.
• [X] I have searched for related issues and found none that matched my issue.
• [X] I have read the FAQ and my problem is not listed.

Playground Link

https://typescript-eslint.io/play/#ts=5.4.5&fileType=.mts&code=MYGwhgzhAEAqCmEAuxL2gbwLAChrQAcBXAIxAEthoB7AJ1gAtbqB3AHgFVp4APJeAHYATGAFFazWgD4AFPAl0AykQIEK8gFzQZASmgBeKdA57sefNGDUByaBGoBbeOMkHuC2stXraugNy4FtBITKx2js4eAeYAvrgxQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6AeyfkNvwAto7AO71og6OiiIx7CeDABfEAqA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

class Testcase {
  public orThrow<U extends Error>(errorSupplier: () => U) {
    const someError = errorSupplier();
    throw someError;
  }
}

ESLint Config

{
  "rules": {
    "@typescript-eslint/only-throw-error": "error"
  }
}

tsconfig

{
  "compilerOptions": {
    "strictNullChecks": true
  }
}

Expected Result

No warning

Actual Result

Expected an error object to be thrown @typescript-eslint/only-throw-error

Additional Info

const someError: U fails, but const someError: Error passes.

Given that U extends Error I expect this to pass as well.

[bradzacher]

You've misunderstood how generic type parameters work in TS.

extends Error does not mean "must be a subclass of Error". Instead it means "must at least be assignable to Error".

For example - this is a valid call for your code:

function orThrow<U extends Error>(errorSupplier: () => U) {
  const someError = errorSupplier();
  throw someError;
}

// This is entirely valid
orThrow(() => ({
  message: '',
  name: '',
}));

playground

Additionally type annotations in TS aren't sound for class-likes. Class-likes are not "nominal" types unless they have a #private property. Without such a property TS allows you to assign objects of the same shape to any argument/variable that's typed as the class.

So "must at least be assignable to Error" really means "must be something with at least the properties message: string and name: string". Not even "a class with at least those properties" -- "an object with at least those properties" would match.

For example here are two CRAZY examples which are valid for your code:

namespace Foo {
  export const name: string = 'a';
  export const message: string = 'b';
}
orThrow(() => Foo);

enum Bar {
    name = "",
    message = "",
}
orThrow(() => Bar);

playground.

Now.. all that being said. The same problems also exist if you typed your function as function orThrow(errorSupplier: () => Error) as well - because all these problems apply in general. So the rule is just weak in this regard -- it's not possible for us to be a whole lot more strict. cc @typescript-eslint/triage-team WDYT? Should the rule accept this case?

The rule in general actually seems pretty weak now that I'm sitting down and looking at it, lol.

[Josh-Cena]

In theory we should only allow nominal subclasses of Error because someone might do e instanceof Error. In practice, you only have two groups: those who use e instanceof Error and use a separate "sad path" that handles everything unknown; and those who assume it's an instance of error and access message and name. Neither of those groups are particularly harmed with duck-typed errors. One thing I may take caution of is if they assume stringification behavior, because this duck-typed error doesn't have a custom toString() but Error instances do.

All this is to say, we should just generally accept types assignable to Error because that's probably the best we could do and it's not going to have extremely bad consequences.