bootloader: only print "failed to execute" error on non-SystemExit errors

[Legorooj]

#4592 added a new traceback to the bootloader. That caused an error that was originally reported and fixed in #1869 and 36b6ab3. The error was that SystemExit - the standard script exit exception, raised by sys.exit and other exit functions, was handled as a normal exception, and the bootloader printed Failed to execute script X, before erroring.

This PR fixes this by checking if this error is a SystemExit first.

Fixes #4860

[Legorooj]

Stuff to do:

• Move the FATALERROR call to after PI_PyErr_Print call when not using windowed mode
• Check if the exception is a SystemExit when in windowed mode, and if so, handle it gracefully before exiting.

[Legorooj]

@bjones1 @danyeaw @htgoebel can I have a hand with this? I'm stuck attemping to batter my way past a bug.

And am I doing the function loading in pyi_python.* correctly?

[Legorooj]

If I add this in, the bootloader doesn't run. Any ideas? Same for the other commented out line above.

[htgoebel]

Thanks for struggling with C :-)

I'm curious about the intention of _handle_system_exit. AFAIU this will terminate the program without traceback if and only if the exception was SystemExit(), SystemExit(0) or SystemExit(None). Esp. the will not terminate on e.g. SystemExit(1), but print the traceback!?

If this understanding is correct, this behaves quite different then the python interpreter and as the non-windows case - which both suppress the traceback on any SystemExit(…).

Also AFAIU this function's purpose is to avoid fetching ptype, pvalue and ptraceback twice. I would not spend code on this optimization, since this only occurs in debug-mod and only if an unexpected error occurs and while the program is terminating anyway.

To summarize: Why not just call PI_PyErr_Print instead of _handle_system_exit? (And if so: Move it above the #if defined(WINDOWED) …)

[htgoebel]

Another idea to simplify the code: The reason for the added #if windows … code ist that in --windowed mode sys.stderr is NullWriter, which will will inhibit printing the traceback. (see #4592 (comment)). So why not simply do something like this:

// scratch
stderr = PySys_GetObject("__stderr__");
PySys_SetObject("stderr", stderr);
PI_PyErr_Print();

This of course requires sys.__stderr__ not being NULL in windowed mode.

[htgoebel]

:-( This will not work, as __stderr__ will be None, too, see https://docs.python.org/3/library/sys.html?highlight=__stderr__#sys.__stderr__

Under some conditions stdin, stdout and stderr as well as the original values __stdin__, __stdout__ and __stderr__ can be None. It is usually the case for Windows GUI apps that aren’t connected to a console and Python apps started with pythonw.

[Legorooj]

@htgoebel I had an idea last night :-). I've moved the PI_PyErr_Print call to before the windowed-only traceback like:

PI_PyErr_Print();
FATALERROR("Failed to execute...");

#if defined(WINDOWED) && defined(LAUNCH_DEBUG)
// Windowed traceback
#endif

My rationale for this change is that when running in windowed mode the PI_PyErr_Print call won't do anything except handle a SystemExit gracefully, because stderr etc are NULL. If we aren't running in windowed mode, then this will work just fine. What do you think? I think this is a lot more graceful than the other method I was using.

Review is out of date due to rework

[htgoebel]

Top!