11

Say I have a promise called myProm, and say I have success and error handlers called onSuccess and onError.

Whenever my promise takes longer than 10 seconds to complete, I want a function called timeoutHandler to be executed, but if that happens, neither onSuccess nor onError should be executed. (Similarly, if either onSuccess or onError runs, I don't want my timeoutHandler to be executed.)

I've come up with the following snippet for this.

new Promise((suc, err) => {
    let outOfTime = false;
    const timeoutId = window.setTimeout(() => {
        outOfTime = true;
        timeoutHandler();
    }, 10000);
    myProm.then(
        (...args) => {
            if (!outOfTime) {
                window.clearTimeout(timeoutId);
                suc(...args);
            }
        },
        (...args) => {
            if (!outOfTime) {
                window.clearTimeout(timeoutId);
                err(...args);
            }
        }
    );
}).then(onSuccess, onError);

However, in case of a timeout, my newly defined promise will be forever-pending. Could this have any negative side effects? For example, the runtime not being able to clean up the Promise object because it's still pending (or something along those lines).

Jessy
  • 311
  • 4
  • 12

2 Answers2

7

There should be no side effect. It would be a browser bug if a non-referenced Promise in whatever state is keeping resources.

Just make sure you don't keep any reference to the Promise object and you'll be fine.

Beware that certain APIs such as setTimeout will keep a reference to the closure up to the timeout value. This means that if you have a long timeout, like the 10s one, you should clear it as soon as you don't need it anymore. Otherwise your code can call thousands of setTimeout within 10s, and each of them will keep a reference to the closure, which in your case will reference the Promise.

fernacolo
  • 7,012
  • 5
  • 40
  • 61
  • But notice that `suc` and `err` do keep the promise alive as well, and `myProm` does keep references to them, so this leads all the way down the rabbit hole… – Bergi Jul 10 '16 at 13:42
4

You can use Promise.race(), set timeoutHandler as a function which returns a rejected a Promise in ten seconds, else onSuccess should be called at fulfilled Promise of myProm

function myProm() {
  return new Promise((success, err) => {
    setTimeout(() => {
      success("myProm")
    }, Math.floor(Math.random() * 11000))
  })
}

function timeoutHandler() {
  return new Promise((_, timeout) => {
    setTimeout(() => {
      timeout(new Error("timeoutHandler"));
    }, 10000)
  })
}

function onSuccess(data) {
  console.log("success", data)
}

function onError(err) {
  console.log("err:", err)
}

function onTimeout(e) {
  if (e.message && e.message === "timeoutHandler") {
    console.log(e.message + " handled");
  }
  else { 
    onError(e)
  }
}

Promise.race([myProm(), timeoutHandler()])
.then(onSuccess, onTimeout);

plnkr http://plnkr.co/edit/9UD5syOEOc1oQGdRTRRm?p=preview

guest271314
  • 1
  • 15
  • 104
  • 177
  • Using `.catch()` chained to `timeoutHandler()`, `throw`, `Boolean` flag, `if` condition to determine whether or not to do stuff within `if` statement at `onError` http://plnkr.co/edit/SmkHN6EPjSQCrnROYWZv?p=preview – guest271314 Jul 10 '16 at 01:50
  • Can you please update the code in your answer so that there are three functions `timeoutHandler, `onSuccess` and `onError` of which exactly one is executed, like the OP requested? – Bergi Jul 10 '16 at 13:44
  • @Bergi See updated post. Not clear from Question or your comment whether error should be propagated to downstream chained `.then()`, `.catch()` or not? – guest271314 Jul 10 '16 at 16:22
  • You're still not calling a `onTimeout`, but if you did replace the `null` with it then it would look good – Bergi Jul 11 '16 at 01:18