Skip to content

Commit 32a7fc0

Browse files
committed
fix try/finally rethrow when try block has return paths
Fixes a regression from #1692 where try/finally (no catch) with both return and throw paths silently lost the error. The re-throw used an undeclared ____error variable; use ____hasReturned instead, which is where pcall places the error on failure.
1 parent c3dc829 commit 32a7fc0

File tree

2 files changed

+62
-6
lines changed

2 files changed

+62
-6
lines changed

src/transformation/visitors/errors.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -159,18 +159,19 @@ export const transformTryStatement: FunctionVisitor<ts.TryStatement> = (statemen
159159
result.push(...context.transformStatements(statement.finallyBlock));
160160
}
161161

162-
// Re-throw error if try had no catch but had a finally
162+
// Re-throw error if try had no catch but had a finally.
163+
// On pcall failure the error is the second return value, which lands in
164+
// ____hasReturned (when functionReturned) or ____error (otherwise).
163165
if (!statement.catchClause && statement.finallyBlock) {
164166
const notTryCondition = lua.createUnaryExpression(
165167
lua.cloneIdentifier(tryResultIdentifier),
166168
lua.SyntaxKind.NotOperator
167169
);
168-
const errorIdentifier = lua.createIdentifier("____error");
170+
const errorIdentifier = tryScope.functionReturned
171+
? lua.cloneIdentifier(returnedIdentifier)
172+
: lua.createIdentifier("____error");
169173
const rethrow = lua.createExpressionStatement(
170-
lua.createCallExpression(lua.createIdentifier("error"), [
171-
lua.cloneIdentifier(errorIdentifier),
172-
lua.createNumericLiteral(0),
173-
])
174+
lua.createCallExpression(lua.createIdentifier("error"), [errorIdentifier, lua.createNumericLiteral(0)])
174175
);
175176
result.push(lua.createIfStatement(notTryCondition, lua.createBlock([rethrow])));
176177
}

test/unit/error.spec.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,61 @@ test("sourceMapTraceback maps anonymous function locations in .lua files (#1665)
417417
expect(result).toContain("<main.ts:2>");
418418
});
419419

420+
test("try/finally rethrow preserves error value", () => {
421+
util.testFunction`
422+
function foo() {
423+
try {
424+
throw "oops";
425+
} finally {
426+
}
427+
}
428+
try { foo(); return "no error"; } catch(e) { return e; }
429+
`.expectToMatchJsResult();
430+
});
431+
432+
test("try/finally with return and throw paths", () => {
433+
util.testFunction`
434+
function foo(shouldReturn: boolean) {
435+
try {
436+
if (shouldReturn) return "returned";
437+
throw "thrown";
438+
} finally {
439+
}
440+
}
441+
const results: any[] = [];
442+
results.push(foo(true));
443+
try { foo(false); } catch(e) { results.push(e); }
444+
return results;
445+
`.expectToMatchJsResult();
446+
});
447+
448+
test("try/finally runs finally side effect before rethrow", () => {
449+
util.testFunction`
450+
let sideEffect = false;
451+
function foo() {
452+
try {
453+
throw "err";
454+
} finally {
455+
sideEffect = true;
456+
}
457+
}
458+
try { foo(); } catch(e) {}
459+
return sideEffect;
460+
`.expectToMatchJsResult();
461+
});
462+
463+
test("try/finally rethrow with non-string error", () => {
464+
util.testFunction`
465+
function foo() {
466+
try {
467+
throw 42;
468+
} finally {
469+
}
470+
}
471+
try { foo(); return "no error"; } catch(e) { return e; }
472+
`.expectToMatchJsResult();
473+
});
474+
420475
util.testEachVersion(
421476
"error stacktrace omits constructor and __TS_New",
422477
() => util.testFunction`

0 commit comments

Comments
 (0)