espruino / Espruino

The Espruino JavaScript interpreter - Official Repo
http://www.espruino.com/
Other
2.75k stars 739 forks source link

`finally` block isn't executed if `try`/`catch` has control flow statements #2338

Closed idobh2 closed 1 year ago

idobh2 commented 1 year ago

First time here, first of all - this is an awesome project!! I'm trying to play around with transpiling async/await code to ES5, and in my attempts I encountered a behavior that's not according to spec. It seems that a finally block are only executed if the try/catch block had no control flow statements in them. For example, in the following code, the finally block should be executed even though the try block had a return statement in it

(() => {
    try {
        console.log("trying...")
        return;
    } finally {
        console.log("this should be written"); // this is not showing in espruino
    }
})();

Running this in node/chrome behaves correctly. MDN and the spec confirm this is indeed the desired behavior.

gfwilliams commented 1 year ago

Thanks for tracking this down!

It seems that a finally block are only executed if the try/catch block had no control flow statements in them.

Is it any control flow statements, like if/whole/do/for/etc, or literally just return that's not working?

idobh2 commented 1 year ago

@gfwilliams It seems that return, break and continue are the problematic statements. I ran tests in the Web IDE using the Bangle.js emulator. These are examples where finally is not executed:

// Not reaching finally block
(() => {
  try {
    console.log("testing return...");
    return;
  } finally {
    console.log("return success");
  }
})();

(() => {
  do {
    try {
      console.log("testing break...");
      break;
    } finally {
      console.log("break success");
    }
  } while (false);
})();

(() => {
  do {
    try {
      console.log("testing continue...");
      continue;
    } finally {
      console.log("continue success");
    }
  } while (false);
})();

And these are cases where it is executed correctly

// Reaching finally block
(() => {
  try {
    console.log("testing if...");
    let someVariable = 0;
    if (true) {
      someVariable = 1;
    }
    if (false) {
      someVariable = 0;
    }
  } finally {
    console.log("if success");
  }
})();

(() => {
  try {
    console.log("testing while...");
    let i = 0;
    while (i < 1) {
      i++;
    }
  } finally {
    console.log("while success");
  }
})();

(() => {
  try {
    console.log("testing do-while...");
    let i = 0;
    do {
      i++;
    } while (i < 2);
  } finally {
    console.log("do-while success");
  }
})();

(() => {
  try {
    console.log("testing for...");
    for (let i = 0; i < 1; i++) {
    }
  } finally {
    console.log("for success");
  }
})();

(() => {
  try {
    console.log("testing throw...");
    throw new Error();
  } catch (e) {
    throw new Error();
  } finally {
    console.log("throw success");
  }
})();

I've confirmed all of them are executed expected in chrome

gfwilliams commented 1 year ago

Thanks! I'm pretty sure that's now fixed.

idobh2 commented 1 year ago

@gfwilliams thanks! confirmed to be working here as well :)