TypeScript 2.1+ and Errors

[benlesh]

Issue

Currently, we're extending Error to create the ability to do instanceof checks again things like ObjectUnsubscribedError. This is no longer something that TypeScript supports for a variety of principled reasons.

Proposed Change

We move to just throwing simple new Error(), but decorating that error with a custom property, and providing a method to check that property. Ideally, that property will be set with a Symbol to avoid collisions.

const timeoutErrorMsg = 'RxJS Timeout Error';
const timeoutErrorSymbol = Symbol(timeoutErrorMsg);

export function createTimeoutError(): Error {
  const error = new Error(timeoutErrorMsg');
  error[timeoutErrorSymbol] = true;
  return error;
}

export function isTimeoutError(err: Error): boolean {
  return err && err[timeoutErrorSymbol];
}

And then to check an error:

source$
  .timeout(3000)
  .catch(err => {
    if (isTimeoutError(err)) {
      // if it's a timeout error from Rx, handle it in some way
      return Observable.of({ handled: 'timeout error' });
    }
    throw err; // otherwise rethrow
  })

Other Things

We should probably make these utilties available on Rx.Util. for example: Rx.Util.isTimeoutError

This affects the following

• EmptyError
• TimeoutError
• ArgumentOutOfRangeError
• UnsubscriptionError
• ObjectUnsubscribedError

This would be a BREAKING CHANGE for sure.

[kwonoj]

#2582

[kwonoj]

So we're not considering mutating Error construction to make custom error object backward compatible like proposed at #2582 (comment) ?

[benlesh]

FWIW: The Angular team has also gone with the approach I mentioned above. I think it's clean and simple. Sucks that it's breaking, but I think it's time we consider releasing a v6 beta.

[benlesh]

@kwonoj ...given that no one's compiler is supporting this, and you'll end up needing to hack around it in ES5 anyhow, we should probably jump on a simple but familiar solution.

[kwonoj]

I see. Let's see if there's some other opinion as well. /cc @david-driscoll too.

[david-driscoll]

I think it makes sense.

We could introduce an interface as well if we need to be able to capture additional information.

Something like...

const timeoutErrorMsg = 'RxJS Timeout Error';
const timeoutErrorSymbol = Symbol(timeoutErrorMsg);

export interface TimeoutError extends Error { hello: string; }

export function createTimeoutError(): TimeoutError {
  const error = new Error(timeoutErrorMsg);
  error[timeoutErrorSymbol] = true;
  return <any>error;
}

export function isTimeoutError(err: Error): err is TimeoutError {
  return err && err[timeoutErrorSymbol];
}

[felixfbecker]

A symbol sounds unnecessarily complex for this. Every other Error in NodeJS just has a code property with known string error codes that can be checked against. Every native error in the browser has a name property. E.g. you can check the error of a fetch like if (err.name === 'AbortError'). To me it is more intuitive to check if name is a specific error code than to check if a certain symbol property is set to true.

Another option is to transpile TypeScript's ES6 output with Babel, which gets the inheritance right. That said, error codes are always a better idea, because any check should work no matter from what version of rxjs the error came from, which is not true for instanceof. Assert on interfaces, not implementations.

[benlesh]

We're well past this now.