Can't narrow error type in catch clause

[yortus]

try {
    ...
}
catch (ex) { // Adding an annotation here gives ERROR TS1196: Catch clause variable cannot have a type annotation
    if (ex instanceof FooError) {
        ex; // ex is of type any
        ex.message; // OK
        ex.iDontExist; // typechecks OK, but doesn't exist
    }
    else if (isBarError(ex)) {
        ex; // ex is of type any
        ex.foo(); // typechecks OK, but runtime error
    }
    else {
        ...
    }
}

I guess this is expected behaviour not a bug. But I was not previously aware that the catch clasue variable cannot be annotated. Due to #1426, this means we can't narrow ex inside the catch clause, so all the cases dealing with specific error types get no type checking (since ex remains as any).

[basarat]

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) 🌹

[yortus]

Thanks @basarat, that's what I'll do for now, and it does get narrowing working. It's a pity to need this hack though :(

[basarat]

It's a pity to need this hack though :(

Basically a throw can come from anywhere and any object can be thrown :-/ :rose:

[yortus]

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.

[yortus]

@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.

[yortus]

Isn't TS1196 an overreach by the compiler anyway? It is possible to know the range of types that can flow into a catch clause in specific code. And if TypeScript cannot infer them, then an annotation on the catch clause variable makes perfect sense (i.e. it provides information to the compiler that it cannot infer by itself, just like annotations do elsewhere). Example:

try {
    noThrow(); // This function's API docs state that it never throws an error
    foo(); // This function's API docs state that it either returns or throws a FooError
}
catch (ex: FooError) {  // ERROR TS1196
    // We could only ever get FooError here
}

[ahejlsberg]

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.

[ahejlsberg]

Now, having said the above, we could perhaps consider allowing type annotations that specify Object or {} since every type is assignable to those types. But I don't think we could go any further than that. With an Object or {} annotation in place, instanceof type guard woulds indeed narrow the type.

[yortus]

we could perhaps consider allowing type annotations that specify Object or {} since every type is assignable to those types

👍 that would at least allow type narrowing and hence better type checking in the catch clause.

[yortus]

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`?
   ...
}

[zpdDG4gta8XKpMCd]

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 {
   /* .. */
}

[yortus]

@Aleksey-Bykov you may be able to recover from some specific error type that you know may be thrown in your try block (so narrowing to that type inside a type guard would be useful), and then re-throw anything else.

[aluanhaddad]

This could lead to code that is very confusing to read.

Catch will catch everything that you might throw at it but the annotation would look like the exception type filter pattern used by many other languages.

In general those languages allow multiple catch clauses to be specified in a manner that can be seen as analogous to type pattern matching, or function overloading.

The critical difference here is that, in said languages, the behavior is generally such that if no catch clause applies to the runtime shape of the exception it is not caught at all.

Well I'm generally opposed to doing things or not doing things simply because they would be intuitive or counterintuitive to programmers coming from other languages, JavaScript explicitly supports only one catch for a reason.

Adding a type annotation would be explicitly misleading because it would intuitively suggest that the exception must be of the designated type to be caught.

However, if there is indeed a need to support different kinds of exceptions being handled differently, you could try creating a library in the vein of Scala's Try type and use user-defined type guards to match different exception types, and then possibly wrap it with a fluent API that implicitly propagates the exception when no handlers match.

[yortus]

@aluanhaddad the point of this issue is not about allowing a potentially confusing annotation, it's about supporting the practice of checking what was thrown in the catch clause, so that for example recoverable errors can be handled and everything else re-thrown.

This is made difficult in TypeScript at present due to the fact that there's currently no way to make type guards work on a variable of type any, which is what the catch variable always is. It would be fine to leave the catch variable unannotated if there was some way of narrowing that variable in type guards in the catch block, but presently there isn't. Unless you count the workaround of introducing a second dummy variable with an annotation, which both adds the 'misleading' annotation you mentioned, as well as introducing an extra variable that is useless at runtime.

[bali182]

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?

[yortus]

@bali182 What you are suggesting would use results from the type system (the e: FooError annotations) to emit different code. TypeScript has a fully-erasable type system, and does not emit different code based on different type annotations. These points mentioned are in the design goals. So I doubt that approach would fly.

[mcclure]

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.

[aluanhaddad]

@mcclure Because code is being generated based solely on types.
consider
app.ts

import { mayThrow, AccessError } from './lib';
try {
    mayThrow();
} catch (e instanceof AccessError) {
    console.error(e.accessViolationMessage)
} catch (e) {
    throw e;
}

lib.d.ts

export declare class AccessError {
    accessViolationMessage: string;
}

export function mayThrow();

lib.js

exports.mayThrow = function () {
    if (true) {
        throw new exports.AccessError();
    }
};

exports.AccessError = function () { 
    var e = new Error(); 
    e.accessViolationMessage = 'Not logged in.'; 
    return e;
};

The error will not be caught, nor the message printed. This code is extremely misleading and the bug will be very hard to track down. See #9999 for a more reasonable alternative.

[mcclure]

@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.

[aluanhaddad]

@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.

[mcclure]

I'm sorry, maybe I am confused :( What is unreliable about the pattern?

It seems like it is completely reliable if you have an accurate d.ts file.

[yortus]

@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.

[aluanhaddad]

@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.

[RyanCavanaugh]

We'll handle this at #9999