Add optional error type param to the Promise

[Gozala]

Unfortunately the way currently promises are typed does not allows authors to type possible errors, which in many cases can be very useful. Furthermore methods like .catch and .then(null, onReject) allow users to recover from possible errors or type out exactly what errors could happen in the resulting chain but again the way promises are typed flow assumes any error type is possible. This is attempt to provide an optional error type parameter so authors can encode error types and flow could infer that most of the time & if error type is not specified it defaults to mixed so that onReject would have to refine error to a specific type rather than assume it to be anything it wants.

Caveat

Given the promise APIs thrown exception could leak through into error type but given that flow does not really handles exceptions I think it is fare to assume that author takes care of encoding error type if exceptions may be thrown. And in other all other cases Promise<T> would do the job.

Next steps

I am willing to do the work to fix up the tests and write new ones to make sure this change works and delivers on it's premise, but I wanted to submit the changes I'd like to make to get an initial feedback & find out whether such a change would be welcome or rejected.

[zerobias]

All unexpected errors in promise chain will be fallen into the .catch argument, so you always will have a significant chance to have an unexpected type at runtime.

The only way to have valid reject type is not to catch synchronous errors; e.g. that is what fluture-js does (and thus has both resolve and reject types)

[Gozala]

All unexpected errors in promise chain will be fallen into the .catch argument, so you always will have a significant chance to have an unexpected type at runtime.

@zerobias I am afraid I don't fully understand your argument. Are you trying to say that possibility of thrown exception make type guarantees here pointless ? If so I disagree, mainly because that is already a case, any function you call might throw exception but flow had proved to be useful never the less.

If you are trying to say that onReject handler has a chance to be invoked with value of arbitrary type that is true, that is why error type is mixed by default. But argument I'm trying to make is that if you do handle error once in your chain then rest of the promise chain can have deterministic type (unless of course exceptions occur in the handler, which is again unfortunate, but I don't think makes this effort pointless).

The only way to have valid reject type is not to catch synchronous errors; e.g. that is what fluture-js does (and thus has both resolve and reject types)

Again I'm not sure what do you mean by synchronous erros, I assume thrown exceptions and this is also what this change proposes. If you're just trying to point out that fantasy land interface is better than standard promise API, I agree, but I also want to make standard API little more type friendly than it currently is. Sad truth is callback based APIs can be better en-typed that promise based ones & I think proposed changes do provide some advantages over just ignoring error type all together.

[zerobias]

is already a case, any function you call might throw exception but flow had proved to be useful never the less

@Gozala Yes, correct flow types are expected behavior itself. When your code fails with the unexpected error, all code below becomes unreachable but still stay correct, so you don't have to think about exceptions to be sure that your types are correct

Example

If anything after a dangerous code is even reachable, logBool argument type will be correct.

declare function logBool(val: boolean): void

function getNavigator(): boolean {
  console.log('before dangerous code')

  const hasNavigator = !!window.navigator // Forgot to check
                                          // that window exists in node.
                                          // throw new ReferenceError

  console.log('after dangerous code')
  return hasNavigator
}

function run() {
  const hasNavigator = getNavigator()
  logBool(hasNavigator) // either correct or not reachable
}

So, "throw smth" is a synchronous error and an unexpected behavior - you could never be sure that your code won't throw in some case.

In fact, it will.

We have that by default, as our code isn't perfect.

Callbacks are easily typed because they have one significant feature, they explicit, you can't unexpectedly send an exception into callback, you always have to call callback function with some object, thereby declaring real function type.

In other words, they not dealing with unexpected synchronous errors and works like your code is always valid, exactly as flow.

Promises will catch every sync error inside them and it is a huge disadvantage for us

declare class Promise<Done, Fail> extends global.Promise<Done> {}

declare function logBool(val: boolean): void

async function getNavigator(): Promise<true, false> {
  console.log('before dangerous code')

  // Forgot to check that window exists in node
  const hasNavigator = !!window.navigator
  console.log('after dangerous code')
  if (!hasNavigator) {
    return Promise.reject(false)
  }
  return hasNavigator
}

function getNavigatorCb(
  cb: (false => void) & ((err: null, result: true) => void)
): void {
  const hasNavigator = !!window.navigator
  hasNavigator
    ? cb(null, true)
    : cb(false)  // Either correct or not reachable
}

function run() {
  getNavigator().catch(
    (result: false) => {
      console.log(`
        It looks like we have false here
        thanks flow to express reject types!`
      )

      logBool(result) // Strongly incorrect
                      // As there will be unexpected ReferenceError
                      // from getNavigator
    }
  )
}

So we have async errors, perfectly typed because of explicit invocation via callbacks and Promise.reject, and sync errors which so weird that often everyone prefers to think that they not even exist. Thus, if we want to type promises, we should act like callbacks and allow sync errors to throws.
At the first time it seems like huge change to solve very small issue, but if you try to work with unsound Promises, you will found that they useless because errors in one places will affect any others which you thought to be trustworthy and you have no way to understand what is real and what is not.

For us, this implies having untyped object.

Surprisingly, when you allow program to fail, you'll have better typed and safer code. But unfortunately, native promises doesn't allow to do that in any way. I didn't understand this too until I tried to safely implement one critical part of the code with them.

BTW, I think that fluture author can explain this better https://github.com/fluture-js/Fluture/wiki/Comparison-to-Promises#error-handling

[SamChou19815]

We are unlikely to pursue this to unnecessarily deviate from TS.