Add --continue-after-exceptions flag; exit on first exception by default #133

New Issue

john · 2025-11-26T14:54:16Z

john commented

2025-11-26 14:54:16 +00:00

Problem Statement

When a Python callback raises an exception (timer, keypress, click, animation), McRogueFace currently:

Prints a warning to stdout
Prints the traceback to stderr via PyErr_Print()
Clears the error and continues running

This behavior is problematic for AI-driven development and automated testing because:

A single exception can produce 1000+ lines of repeated output as the timer fires repeatedly
The exception is "swallowed" - the process exits with code 0 even though errors occurred
In headless/automated mode, there's no benefit to continuing after an exception

Current Exception Handling Locations

File	Line	Callback Type	Has Warning Message
`Timer.cpp`	53-57	Timer callbacks	✅ Yes
`PyCallable.cpp`	52-56	Click callbacks	✅ Yes
`PyCallable.cpp`	82-86	Key callbacks	✅ Yes
`Animation.cpp`	370-373	Animation callbacks	❌ No (silent)

All use the pattern:

if (!retval) {
    std::cout << "... has raised an exception..." << std::endl;
    PyErr_Print();
    PyErr_Clear();
}

Proposed Solution

New Default Behavior

On the first unhandled exception in any callback:

Print the traceback (as currently)
Terminate the game loop cleanly
Exit with non-zero status (e.g., exit code 1)

New Flag: `--continue-after-exceptions`

Preserves current behavior for developers who want to:

Use the REPL to inspect state after an error
Keep the game running to work around errors interactively
Debug while the program continues

Implementation Changes

1. Add to `McRogueFaceConfig.h`:

bool continue_after_exceptions = false;  // New default: exit on first exception

2. Add to `CommandLineParser.cpp`:

if (arg == "--continue-after-exceptions") {
    config.continue_after_exceptions = true;
    current_arg++;
    continue;
}

3. Update help text in `print_help()`:

--continue-after-exceptions : don't exit on Python callback exceptions (default: exit on first exception)

4. Create exception signaling mechanism

Option A: Global atomic flag (simplest)

// In new file ExceptionHandler.h
#include <atomic>
extern std::atomic<bool> g_exception_occurred;
extern std::atomic<int> g_exit_code;

void signalPythonException();  // Sets flag and exit code
bool shouldExitOnException();  // Checks config

Option B: Singleton accessor

// Access via McRFPy_API or GameEngine static method
GameEngine::signalFatalException();

5. Modify exception handlers (example for Timer.cpp):

if (!retval) {
    std::cerr << "Timer callback raised an exception:" << std::endl;
    PyErr_Print();
    PyErr_Clear();
    
    if (!McRFPy_API::getConfig().continue_after_exceptions) {
        signalPythonException();  // Signal game loop to exit
    }
}

6. Check flag in game loop (`GameEngine.cpp`):

while (running) {
    // ... existing loop code ...
    
    if (g_exception_occurred) {
        running = false;
    }
}
// After loop exits, return g_exit_code

Exit Code Behavior

Scenario	Current	Proposed
Clean exit	0	0
`sys.exit(0)`	0	0
`sys.exit(1)`	1	1
Unhandled exception (default)	0	1
Unhandled exception + `--continue-after-exceptions`	0	0

Testing Considerations

Existing tests that rely on sys.exit(0) for PASS should continue working
Tests with exceptions will now fail properly instead of timing out
May need to update test runner timeout handling

Alternatives Considered

Per-callback-type flags: Too complex, low benefit
Exception count limit: Arbitrary, doesn't solve the core problem
Only in headless mode: Inconsistent behavior is confusing

Test output flooding observed in test_python_object_cache.py failure
Affects all timer-based tests that encounter exceptions

## Problem Statement When a Python callback raises an exception (timer, keypress, click, animation), McRogueFace currently: 1. Prints a warning to stdout 2. Prints the traceback to stderr via `PyErr_Print()` 3. Clears the error and **continues running** This behavior is problematic for AI-driven development and automated testing because: - A single exception can produce **1000+ lines of repeated output** as the timer fires repeatedly - The exception is "swallowed" - the process exits with code 0 even though errors occurred - In headless/automated mode, there's no benefit to continuing after an exception ## Current Exception Handling Locations | File | Line | Callback Type | Has Warning Message | |------|------|---------------|---------------------| | `Timer.cpp` | 53-57 | Timer callbacks | ✅ Yes | | `PyCallable.cpp` | 52-56 | Click callbacks | ✅ Yes | | `PyCallable.cpp` | 82-86 | Key callbacks | ✅ Yes | | `Animation.cpp` | 370-373 | Animation callbacks | ❌ No (silent) | All use the pattern: ```cpp if (!retval) { std::cout << "... has raised an exception..." << std::endl; PyErr_Print(); PyErr_Clear(); } ``` ## Proposed Solution ### New Default Behavior On the **first unhandled exception** in any callback: 1. Print the traceback (as currently) 2. **Terminate the game loop** cleanly 3. **Exit with non-zero status** (e.g., exit code 1) ### New Flag: `--continue-after-exceptions` Preserves current behavior for developers who want to: - Use the REPL to inspect state after an error - Keep the game running to work around errors interactively - Debug while the program continues ### Implementation Changes #### 1. Add to `McRogueFaceConfig.h`: ```cpp bool continue_after_exceptions = false; // New default: exit on first exception ``` #### 2. Add to `CommandLineParser.cpp`: ```cpp if (arg == "--continue-after-exceptions") { config.continue_after_exceptions = true; current_arg++; continue; } ``` #### 3. Update help text in `print_help()`: ``` --continue-after-exceptions : don't exit on Python callback exceptions (default: exit on first exception) ``` #### 4. Create exception signaling mechanism Option A: **Global atomic flag** (simplest) ```cpp // In new file ExceptionHandler.h #include <atomic> extern std::atomic<bool> g_exception_occurred; extern std::atomic<int> g_exit_code; void signalPythonException(); // Sets flag and exit code bool shouldExitOnException(); // Checks config ``` Option B: **Singleton accessor** ```cpp // Access via McRFPy_API or GameEngine static method GameEngine::signalFatalException(); ``` #### 5. Modify exception handlers (example for Timer.cpp): ```cpp if (!retval) { std::cerr << "Timer callback raised an exception:" << std::endl; PyErr_Print(); PyErr_Clear(); if (!McRFPy_API::getConfig().continue_after_exceptions) { signalPythonException(); // Signal game loop to exit } } ``` #### 6. Check flag in game loop (`GameEngine.cpp`): ```cpp while (running) { // ... existing loop code ... if (g_exception_occurred) { running = false; } } // After loop exits, return g_exit_code ``` ## Exit Code Behavior | Scenario | Current | Proposed | |----------|---------|----------| | Clean exit | 0 | 0 | | `sys.exit(0)` | 0 | 0 | | `sys.exit(1)` | 1 | 1 | | Unhandled exception (default) | 0 | **1** | | Unhandled exception + `--continue-after-exceptions` | 0 | 0 | ## Testing Considerations - Existing tests that rely on `sys.exit(0)` for PASS should continue working - Tests with exceptions will now fail properly instead of timing out - May need to update test runner timeout handling ## Alternatives Considered 1. **Per-callback-type flags**: Too complex, low benefit 2. **Exception count limit**: Arbitrary, doesn't solve the core problem 3. **Only in headless mode**: Inconsistent behavior is confusing ## Related - Test output flooding observed in `test_python_object_cache.py` failure - Affects all timer-based tests that encounter exceptions

john added the

Alpha Release Requirement

label 2025-11-26 14:54:23 +00:00

john added

Minor Feature

system:python-binding

priority:tier1-active

and removed

Alpha Release Requirement

labels 2025-11-26 15:14:27 +00:00

john referenced this issue from a commit

2025-11-26 16:16:34 +00:00