Transcrypt icon indicating copy to clipboard operation
Transcrypt copied to clipboard

`else` block in `try` statement swallows exceptions

Open mentalisttraceur opened this issue 6 years ago • 4 comments

Exceptions raised in the else block should bubble out, but instead they currently get suppressed.

This:

try:
    pass
except:
    pass
else:
    raise Exception('something wrong happened in the `else`')

compiles to:

try {
        // pass;
        try {
                var __except0__ = Exception ('something wrong happened in the `else`');
                __except0__.__cause__ = null;
                throw __except0__;
        }
        catch (__except0__) {
        }
}
catch (__except0__) {
        // pass;
}

I suggest: set a temporary boolean variable to true before the try, then flip it to false as the very first statement inside the catch, and then have an if statement on that boolean as the very first statement after the catch block (it might be better to just always generate that if inside a finally in the JavaScript even if there is no finally in the Python, because it has to execute before any finally if it does exist).

(My reasoning for flipping the boolean as the first statement in the catch instead of the last statement in the try is that if for some reason ever a JavaScript implementation existed that raised an exception while trying to set that internal implementation book-keeping variable, I think it better that our generated code explode that up a level, instead of it being caught by the local except block, because logically that is an error within the try statement itself, not the block of code inside it.)

P.S. In similar situations in the future, would you prefer I reopened #99 instead of creating this new issue?

mentalisttraceur avatar Feb 20 '19 10:02 mentalisttraceur

To be clear, what I'm saying is that the fix for this bug would be as simple as generating code like this instead:

(Edit: this example has a bug, see next comment)

var __else__ = true;
try {
        // try body
}
catch (__except0__) {
        __else__ = false;
        // except clauses
}
finally {
    if (__else__) {
        // else body
    }
    // finally body
}

I'm happy to submit a pull request to that effect, though it would take me some time to familiarize myself with the code enough to do so.

mentalisttraceur avatar Feb 27 '19 01:02 mentalisttraceur

I realized there is a bug with my proposed fix too: Exceptions raised in the else block would prevent the rest of the finally from executing.

So my fixed proposal is to generate code like this:

try {
    var __else__ = true;
    try {
        // try body
    }
    catch (__except0__) {
        __else__ = false;
        // except clauses
    }
    if (__else__) {
        // else body
    }
}
finally {
    // finally body
}

I had hoped to avoid two nested try blocks, but it seems that this is the least bad option.

mentalisttraceur avatar May 22 '19 20:05 mentalisttraceur

@JdeH does the "under consideration" label mean "considering whether or not this should be done at all" or "considering how best to do it" in this case?

mentalisttraceur avatar Dec 07 '19 04:12 mentalisttraceur

Oh, one more fix to avoid breakage when nesting two or more try blocks, each with an else:

If we're embracing ES6 features, we should just replace var with let:

try {
    let __else__ = true;
    try {
        // try body
    }
    catch (__except0__) {
        __else__ = false;
        // except clauses
    }
    if (__else__) {
        // else body
    }
}
finally {
     // finally body
}

This fixes this edge case misbehavior in my last proposal:

try:
    try:
        raise Exception('boom')
    else:
        pass
else:
    print("should've printed!")

(If you want to still generate older JS, I would give each try in the same scope, or at least each level of nested try in the same scope, its own numbered "else" variable: __else0__, __else1__, etc.)

mentalisttraceur avatar Jun 10 '22 22:06 mentalisttraceur