From 9b011979b1477578d7bef62661180268622539b5 Mon Sep 17 00:00:00 2001
From: "Remi GASCOU (Podalirius)"
<79218792+p0dalirius@users.noreply.github.com>
Date: Sat, 18 Apr 2026 09:50:40 +0200
Subject: [PATCH] Fix ON TIMEOUT/ON ERROR handlers not firing from inside a
function (#3)
---
CMakeLists.txt | 1 +
include/5250script/script_executor.h | 6 ++
src/script_executor.cpp | 16 ++++
tests/test_script_executor.cpp | 106 +++++++++++++++++++++++++++
4 files changed, 129 insertions(+)
create mode 100644 tests/test_script_executor.cpp
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 035fb19..a921a7b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -51,4 +51,5 @@ if(BUILD_5250SCRIPT_TESTS)
add_5250script_test(test_script_lexer tests/test_script_lexer.cpp)
add_5250script_test(test_script_parser tests/test_script_parser.cpp)
+ add_5250script_test(test_script_executor tests/test_script_executor.cpp)
endif()
diff --git a/include/5250script/script_executor.h b/include/5250script/script_executor.h
index 03436fd..76386dc 100644
--- a/include/5250script/script_executor.h
+++ b/include/5250script/script_executor.h
@@ -120,6 +120,12 @@ class ScriptExecutor : public QObject {
// GOTO support (only at root level)
void gotoLabel(const QString &label);
+ // Unwind the call stack and any nested exec frames so that the runtime
+ // is back at top-level script context. Used when ON TIMEOUT / ON ERROR
+ // handlers fire from inside a function: the handler label is, by
+ // construction, top-level, so we must exit the function first.
+ void unwindToTopLevel();
+
// Error handlers
QString m_onTimeoutLabel;
QString m_onErrorLabel;
diff --git a/src/script_executor.cpp b/src/script_executor.cpp
index 13fd9e1..9f29f3a 100644
--- a/src/script_executor.cpp
+++ b/src/script_executor.cpp
@@ -119,6 +119,9 @@ void ScriptExecutor::notifyTerminalStateChanged() {
// Terminal entered error-locked state — trigger ON ERROR handler if set
m_waitingForUnlock = false;
if (!m_onErrorLabel.isEmpty()) {
+ // If the error happened inside a function, exit the function first
+ // so that gotoLabel (which only accepts top-level labels) can dispatch.
+ unwindToTopLevel();
gotoLabel(m_onErrorLabel);
scheduleNextStep();
} else {
@@ -612,6 +615,9 @@ void ScriptExecutor::endExpect(bool success) {
} else {
setVariable("$EXPECT_RESULT", "TIMEOUT");
if (!m_onTimeoutLabel.isEmpty()) {
+ // If the timeout happened inside a function, exit the function first
+ // so that gotoLabel (which only accepts top-level labels) can dispatch.
+ unwindToTopLevel();
gotoLabel(m_onTimeoutLabel);
scheduleNextStep();
} else {
@@ -748,6 +754,16 @@ void ScriptExecutor::gotoLabel(const QString &label) {
m_execStack.append({&m_parseResult.root->children, targetIndex, 0, 0});
}
+void ScriptExecutor::unwindToTopLevel() {
+ while (!m_callStack.isEmpty()) {
+ // Pop any exec frames opened inside the current function (nested IF/WHILE/REPEAT bodies)
+ while (m_execStack.size() > m_callStack.last().execStackDepth) {
+ m_execStack.removeLast();
+ }
+ returnFromFunction();
+ }
+}
+
// --- Function return ---
void ScriptExecutor::returnFromFunction() {
diff --git a/tests/test_script_executor.cpp b/tests/test_script_executor.cpp
new file mode 100644
index 0000000..ddf18e3
--- /dev/null
+++ b/tests/test_script_executor.cpp
@@ -0,0 +1,106 @@
+// 5250ng - A modern IBM TN5250 terminal emulator
+// Copyright (C) 2025-2026 Remi GASCOU (Podalirius)
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see .
+
+#include <5250script/screen_interface.h>
+#include <5250script/script_executor.h>
+#include <5250script/script_parser.h>
+#include
+#include
+
+using namespace core::scripting;
+
+// Minimal ScreenInterface stub for executor tests.
+class FakeScreen : public ScreenInterface {
+ public:
+ int rows() const override { return m_rows; }
+ int cols() const override { return m_cols; }
+ int cursorRow() const override { return 0; }
+ int cursorCol() const override { return 0; }
+ QString readText(int, int, int length) const override { return QString(length, ' '); }
+ QString readFieldText(int, int) const override { return {}; }
+ KeyboardState keyboardState() const override { return m_state; }
+ bool messageWaiting() const override { return false; }
+
+ void setKeyboardState(KeyboardState s) { m_state = s; }
+
+ private:
+ int m_rows = 24;
+ int m_cols = 80;
+ KeyboardState m_state = KeyboardState::Unlocked;
+};
+
+class TestScriptExecutor : public QObject {
+ Q_OBJECT
+
+ private slots:
+ void onTimeoutFromInsideFunction();
+};
+
+// Regression test for #3: ON TIMEOUT must fire even when the EXPECT that
+// times out was entered from within a CALL-ed function. Previously the
+// handler was suppressed because gotoLabel() refused to run with a non-empty
+// call stack, terminating the script with "GOTO is not allowed inside
+// functions" instead of dispatching to the user-registered handler.
+void TestScriptExecutor::onTimeoutFromInsideFunction() {
+ FakeScreen screen;
+ ScriptExecutor executor;
+ executor.setScreen(&screen);
+
+ const QString source =
+ "GLOBAL EXPECT_TIMEOUT 50\n"
+ "ON TIMEOUT GOTO handler\n"
+ "CALL f()\n"
+ "LOG \"should-not-run\"\n"
+ "LABEL handler\n"
+ "LOG \"caught\"\n"
+ "\n"
+ "DEF f()\n"
+ " EXPECT TEXT \"never-present\"\n"
+ "ENDDEF\n";
+
+ ScriptParser parser;
+ auto result = parser.parse(source);
+ QVERIFY(!result.hasErrors());
+
+ QSignalSpy logSpy(&executor, &ScriptExecutor::logMessage);
+ QSignalSpy errorSpy(&executor, &ScriptExecutor::executionError);
+ QSignalSpy finishedSpy(&executor, &ScriptExecutor::executionFinished);
+
+ executor.execute(result);
+
+ QVERIFY(finishedSpy.wait(5000));
+
+ // The handler must have run — LOG "caught" should appear.
+ bool caughtLogged = false;
+ bool shouldNotRunLogged = false;
+ for (const auto &args : logSpy) {
+ const QString msg = args.at(0).toString();
+ if (msg == "caught") caughtLogged = true;
+ if (msg == "should-not-run") shouldNotRunLogged = true;
+ }
+ QVERIFY(caughtLogged);
+ QVERIFY(!shouldNotRunLogged);
+
+ // No "GOTO is not allowed inside functions" error should be emitted.
+ for (const auto &args : errorSpy) {
+ const QString msg = args.at(1).toString();
+ QVERIFY2(!msg.contains("GOTO is not allowed inside functions"),
+ qPrintable(QString("unexpected error: %1").arg(msg)));
+ }
+}
+
+QTEST_MAIN(TestScriptExecutor)
+#include "test_script_executor.moc"