TypeScript: Can't narrow error type in catch clause

Forgive me, if I this has been discussed before, but whats wrong with doing something like this:

try {
  // ...
} catch (e: FooException) {
  // ...
} catch (e: BarException) {
  // ...
} catch (e) {
  // ...
}

Which would generate JS like this:

try {
  // ...
} catch (e) {
  if (e instanceof FooException) {
    // do stuff in foo block
  } else if (e instanceof BarException) {
    // do bar block
  } else {
    // catch all block
  }
}

If there is no catch all block, then just re-throw the exception:

try {
  // ...
} catch (e: FooException) {
  // ...
}

JS:

try {
  // ...
} catch (e) {
  if (e instanceof FooException) {
    // do stuff in foo block
  } else {
    throw e;
  }
}

And in the type annotations would maybe only accept constructors? So what’s “instanceof-able” in JS? Maybe also string, number, and whatever is easily type-checkable with typeof?

Two interrelated issues being discussed here:

• We don’t allow type annotations on catch clauses because there’s really no way to know what type an exception will have. You can throw objects of any type and system generated exceptions (such as out of memory exception) can technically happen at any time. Even if we had a Java-like notion of throws annotations it is not clear that we could ever really rely on them. (And, it is also not clear that they work all that well in Java, but that’s another discussion.)
• We don’t narrow any in type guards unless we know the _exact_ type you’re narrowing to. For example, if you check typeof x === "string" we will indeed narrow an any to string because we know for sure that is the exact type of x. That isn’t the case with instanceof. Specifically, you might check x instanceof Base, but x might actually be an instance of a class derived from Base. If we narrowed x to type Base you’d see errors on access to properties of the derived class even though there is nothing wrong with the code. Now, before you argue those errors would be fine, keep in mind that the declared type of x is any, so you’ve already said you want no checking. We take that as an indication that we should issue errors only when we’re 100% sure they’re errors. And we only know that if we know the exact type.

@mcclure Personally I don’t think your suggestions are ‘unreliable’ any more or less that other patterns in TypeScript which can also be made unreliable if we try hard enough.

The thing that makes @bali182’s original suggestion likely incompatible with TypeScript’s design goals is that the following three examples only differ by a type annotation, but all would generate different JS code with different runtime behaviour:

try { foo(); } catch (ex) {/***/}

try { foo(); } catch (ex: Error) {/***/}

try { foo(); } catch (ex: FooError) {/***/}

Type annotations are there to inform the compiler about what the program actually does, but they never change what it does. With these ‘hints’, the type checker can detect invalid usage at compile-time and produce useful early feedback to the developer. If you strip out all the type annotations in a program, that would not change the meaning/behaviour of the program. But it would in the above example.

Your other suggestion (multiple catch (e instanceof X) {...} blocks) is better in the sense that it doesn’t rely on type annotations. But it’s still a big ask, because it introduces new syntax that has no corresponding ECMA proposal (IFAIK). Apart from adding types to JavaScript, TypeScript has pioneered a few other kinds of new syntax (class properties, modules, namespaces, async/await), but generally only ones that either have an ECMA proposal in development, or represent a very widespread idiom (eg namespaces are just JavaScript’s ubiquitous ‘revealing module pattern’).

But mainly, the problems in this issue would simply vanish if TypeScript just allowed the catch variable to be narrowed. We already have type guards and control flow analysis. I think there’s little need to add new syntax for this one special case, when everything is already provided to write idiomatic code, if only the catch variable could be narrowed:

try {
    foo();
}
catch (ex) {
    if (ex instanceof FooError) {
        /* recover from FooError */
        ex.foo
        ...
    }
    else {
        throw ex;
    }
}

Thas already works at runtime and compiles without errors too, but is just not typesafe because ex remains typed as any thoughout so no typos will be found, and no refactorings will find symbol references in this code.

But I was not previously aware that the catch clasue variable cannot be annotated.

The workaround is to simply create a local variable:

catch (err) {
    const ex:Error = err;

However it will not help with your desire of narrowing in any meaningful way (I think) 🌹

I was about to file something about this but I see there’s already an issue…

I would like to frame the problem this way. This is a really standard Javascript idiom (I’ve seen it recommended in pure-JS stackoverflow answers more than once; here’s one example):

try {
    throw new CustomError(3)
} catch (e) {
    if (e instanceof CustomError)
        console.log(e.ocde)
    else
        throw e
}

Not only is it a common idiom, because of the limitations of Javascript as a language there is no better way to do it if you are doing exceptions with ES6 classes. In all other cases I’ve seen, Typescript takes common Javascript idioms and finds some way to make them typesafe, even if the result is a little awkward. However Typescript fails in this case to give me a convenient way to make a common, mandatory even, part of Javascript programming typeable. I think this means Typescript is not living up to its promise in this case, and it should be fixed somehow. That could mean type-narrowing on if ( x isinstance e ), it could mean a special ifinstance(x, e) or an explicitly narrowing variant of isinstance, it could mean bali182’s proposal.

I do like bali182’s proposal best because it would produce more convenient code that looks like other languages. It would shorten the simple catch clause in my example from six lines (eight, if I choose to introduce a second typed variable for e) to three. I don’t agree with the idea this interferes with Typescript’s goal of being an erasing compiler. There are other situations where Typescript introduces a syntax construct completely absent from javascript-- enum {} would be an example-- if it is necessary to fill out the type system. Multiple catch{}es are not legal Javascript and typed catches are not legal Typescript so this is not altering behavior based on type so much as introducing a new syntax construct that compiles to idiomatic Javascript. If there’s something philosophically unpleasant about the fact that what triggers the multi-catch is specifically a type annotation, maybe the syntax could be catch (e instanceof CustomException) instead of catch (e : CustomException) or something else to make it very clear this is a TypeScript specific construct.

Incidentally, there is a conditional catch clause syntax Firefox has introduced as a js extension and which may be relevant to be aware of here. However it is not on a standards track, and it is also very ugly.

It would be very nice if we could type-annotate the catch clause variable with ex: Error since it’s perfectly sound to do so in many if not most cases. Consider:

• System-generated exceptions (including out of memory if that indeed exists and can be caught) will always be instances of Error or one of its a subclasses such as ReferenceError.
• All other exceptions come from an explicit throw statement somewhere in our own code or in third party code, that is executed within the guarded try block.
• I control my own code so I know that it only ever throws Error instances.
• I have try blocks that contain no third party code.
• I have try blocks that reference third party libraries that very explicitly document that they only throw Error instances.

---

The counter-argument that TypeScript should disallow the annotation in all circumstances because it could be incorrect in some circumstances is an interesting one:

try {
   throw new Error("I'm an error");
}
catch (ex: Error) {  // ERROR: We won't allow this annotation even though it's valid
                     // in this case, because it might be invalid in some other case
   ...
}

…given that the obvious workaround is to exploit the “user knows best” philosophy that TypeScript applies to an equivalent annotation in a different position:

try {
   throw new Error("I'm an error");
}
catch (ex) {
    let err: Error = ex;  // OK, user knows best! And indeed in this case they are correct.
                          // But if we trust the user then why not just allow annotating `ex`?
   ...
}

@mcclure I am saying that instanceof is not reliable. instanceof is not a type test, it is a value test. It performs a reference comparison between the prototype properties of its operands. Because prototypes are mutable, any object can have its heritage rewired.

var x = { name: 'Bob', age: 35 };
Reflect.setPrototypeOf(x, Date.prototype);

console.log(x instanceof Date) // true

x.getHours(); // Error

@yortus you are correct that there plenty of other patterns that can be unreliable, but TypeScript is fairly agnostic with respect to whether you use them. As you point out this is a big thing to ask for, in other words, it is a new language feature. Adding a whole new language feature around instanceof would make the pattern seem clandestine. Regardless, I agree with your analysis that allowing type narrowing of catch variables makes sense.

@mcclure you’re absolutely correct that this suffers from the same problems. The implication is that instanceof is not reliable. Personally, I think it is an anti-pattern.

The problem I have with @bali182’s suggested solution is that it bakes an unreliable pattern into the language, encouraging its use by providing manifest syntax, and providing a false sense of security to the programmer.

With #9999 a restriction is being lifted, but you still have to choose to shoot yourself in the foot and nothing encourages you to do so.

@aluanhaddad So, in your example, it seems like the Typescript type checker only does something incorrect because the ambient typings are incorrect. It seems like by nature Typescript will typecheck incorrectly if its ambient type declarations are incorrect.

This said, Typescript does not really do anything incorrect in your example. e is not an instance of AccessError, so it is correct the the AccessError clause does not trigger. The code itself has a bug-- but you would get the same bug, and it would be just as hard to find, if you were using plain JS instead of Typescript and the idiomatic Javascript catch (e) { if (e instanceof AccessError) { console.log("...") } } were used. (You could wind up writing this code if the documentation for the module exporting AccessError were incorrect, for example.) I do not see the generated behavior here as based on “a Type”; the behavior is based on CustomError in its capacity as a symbol in scope.

Moreover, we would have all of these same problems both with the buggy behavior and the type checking if we wrote the idiomatic Javascript in a scenario where the narrowing instanceof (bug 9999 you reference) were implemented. If bug 9999 were implemented and one used the e instanceof AccessError…else throw idiom with your code, e would incorrectly type as AccessError within the narrowed scope. So although narrowing-instanceof sounds like a great proposal to me, I do not think your example is a argument that narrowing-instanceof is specifically superior to bali182’s proposal. In fact, if the ambient types in your example were accurate (marking AccessError as a () => Error), then I think bali182’s proposal would perform slightly better on your example scenario than narrowing-instanceof. Narrowing-instanceof would allow the e instanceof AccessError (because things of type () => Error are valid right-operands to instanceof) and then just silently fail to type e inside the if scope. Your sample code would throw an error, but only because it happened to use one of the fields of what the code author believed AccessError to be. Bali182’s proposal on the other hand is a more constrained construct, and so it could check more aggressively and identify that } catch (e: AccessError) { is itself invalid code because AccessError is not something which can ever be a type.

once the exception caught what is that that you are going to do with it? a rhetorical question, but at the end of the day your options are limited:

• an unexpected exception which is absolutely nothing you can do about, extremely hard to reliably recover, takes special coding discipline to do so, just fail fast and loud
• an expected exception, with a chance for graceful recovery, if so, pardon my curiosity, why would you use exceptions (that are terribly supported by the JS runtime) if you can make it a legit part of the result value type?

function readFileSync(path: string, encoding: string): string | FileDoesNotExist | FileIsLocked {
   /* .. */
}

@basarat BTW the hack I’m referring to is the need for both err and ex to get the job done. Someone reading this code (or, say, debugging the output js) would not understand why the second variable is required, without knowing something about the contentious history of narrowing with any.

EDIT: -And- the workaround with both err and ex is subject to the same fallacy you mention about assuming an instance of Error.

Yep, that part i agree with. What bothers me is the fact that narrowing doesn’t work with any, and there’s no alternative to using any here.