fix try/finally rethrow when try block has return paths (#1699)

RealColdFry · web-flow · commit ec26bdf4e15b · 2026-03-29T20:46:48.000+02:00
* 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. * add test for try/finally with return+throw and non-empty finally body
diff --git a/src/transformation/visitors/errors.ts b/src/transformation/visitors/errors.ts
@@ -159,18 +159,19 @@ export const transformTryStatement: FunctionVisitor<ts.TryStatement> = (statemen
         result.push(...context.transformStatements(statement.finallyBlock));
     }
 
-    // Re-throw error if try had no catch but had a finally
+    // Re-throw error if try had no catch but had a finally.
+    // On pcall failure the error is the second return value, which lands in
+    // ____hasReturned (when functionReturned) or ____error (otherwise).
     if (!statement.catchClause && statement.finallyBlock) {
         const notTryCondition = lua.createUnaryExpression(
             lua.cloneIdentifier(tryResultIdentifier),
             lua.SyntaxKind.NotOperator
         );
-        const errorIdentifier = lua.createIdentifier("____error");
+        const errorIdentifier = tryScope.functionReturned
+            ? lua.cloneIdentifier(returnedIdentifier)
+            : lua.createIdentifier("____error");
         const rethrow = lua.createExpressionStatement(
-            lua.createCallExpression(lua.createIdentifier("error"), [
-                lua.cloneIdentifier(errorIdentifier),
-                lua.createNumericLiteral(0),
-            ])
+            lua.createCallExpression(lua.createIdentifier("error"), [errorIdentifier, lua.createNumericLiteral(0)])
         );
         result.push(lua.createIfStatement(notTryCondition, lua.createBlock([rethrow])));
     }
diff --git a/test/unit/error.spec.ts b/test/unit/error.spec.ts
@@ -417,6 +417,82 @@ test("sourceMapTraceback maps anonymous function locations in .lua files (#1665)
     expect(result).toContain("<main.ts:2>");
 });
 
+test("try/finally rethrow preserves error value", () => {
+    util.testFunction`
+        function foo() {
+            try {
+                throw "oops";
+            } finally {
+            }
+        }
+        try { foo(); return "no error"; } catch(e) { return e; }
+    `.expectToMatchJsResult();
+});
+
+test("try/finally with return and throw paths", () => {
+    util.testFunction`
+        function foo(shouldReturn: boolean) {
+            try {
+                if (shouldReturn) return "returned";
+                throw "thrown";
+            } finally {
+            }
+        }
+        const results: any[] = [];
+        results.push(foo(true));
+        try { foo(false); } catch(e) { results.push(e); }
+        return results;
+    `.expectToMatchJsResult();
+});
+
+test("try/finally runs finally side effect before rethrow", () => {
+    util.testFunction`
+        let sideEffect = false;
+        function foo() {
+            try {
+                throw "err";
+            } finally {
+                sideEffect = true;
+            }
+        }
+        try { foo(); } catch(e) {}
+        return sideEffect;
+    `.expectToMatchJsResult();
+});
+
+test("try/finally with return and throw paths and non-empty finally body", () => {
+    util.testFunction`
+        let sideEffect = false;
+        function foo(shouldReturn: boolean) {
+            try {
+                if (shouldReturn) return "ok";
+                throw "err";
+            } finally {
+                sideEffect = true;
+            }
+        }
+        const results: any[] = [];
+        results.push(foo(true));
+        results.push(sideEffect);
+        sideEffect = false;
+        try { foo(false); } catch(e) { results.push(e); }
+        results.push(sideEffect);
+        return results;
+    `.expectToMatchJsResult();
+});
+
+test("try/finally rethrow with non-string error", () => {
+    util.testFunction`
+        function foo() {
+            try {
+                throw 42;
+            } finally {
+            }
+        }
+        try { foo(); return "no error"; } catch(e) { return e; }
+    `.expectToMatchJsResult();
+});
+
 util.testEachVersion(
     "error stacktrace omits constructor and __TS_New",
     () => util.testFunction`
