OOB Failure - Example Does not Run on Windows

[thygrrr]

I would like to report a pair of out-of-the-box issues with example.py / the "SDK".

In a Python 3.10.2 venv on Windows 11 Pro x64, trying to run the example will yield the following output:

C:\Projects\neurosity-python-sdk\venv\Scripts\python.exe C:\Projects\neurosity-python-sdk\examples\example.py 
Traceback (most recent call last):
  File "C:\Projects\neurosity-python-sdk\examples\example.py", line 7, in <module>
    neurosity = NeurositySDK({
  File "C:\Projects\neurosity-python-sdk\neurosity\neurosity.py", line 27, in __init__
    signal.signal(signal.SIGHUP, self.exit_handler)
AttributeError: module 'signal' has no attribute 'SIGHUP'
Exception ignored in atexit callback: <bound method NeurositySDK.exit_handler of <neurosity.neurosity.NeurositySDK object at 0x0000020CBA085B70>>
Traceback (most recent call last):
  File "C:\Projects\neurosity-python-sdk\neurosity\neurosity.py", line 30, in exit_handler
    self.remove_client()
  File "C:\Projects\neurosity-python-sdk\neurosity\neurosity.py", line 61, in remove_client
    client_id = self.client_id
AttributeError: 'NeurositySDK' object has no attribute 'client_id'

Process finished with exit code 1

Issue diagnosis

The main issue is that it appears as if the signal library is not used correctly in a portable way. For platform portability, only signals from dir(signal) are valid, or at runtime, signal.valid_signals().

On Windows 11 pro x64, this notably excludes SIGHUP, which is only available on Unix and on most BSD flavors (including OSX).

Python 3.10.2 (tags/v3.10.2:a58ebcc, Jan 17 2022, 14:12:15) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import signal
>>> dir(signal)
['CTRL_BREAK_EVENT', 'CTRL_C_EVENT', 'Handlers', 'NSIG', 'SIGABRT', 'SIGBREAK', 'SIGFPE', 'SIGILL', 'SIGINT', 'SIGSEGV', 'SIGTERM', 'SIG_DFL', 'SIG_IGN', 'Signals', '_IntEnum', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__', '_enum_to_int', '_int_to_enum', '_signal', 'default_int_handler', 'getsignal', 'raise_signal', 'set_wakeup_fd', 'signal', 'strsignal', 'valid_signals']

The other issue show that some failure state cannot be handled, probably because it uses the dangerous JavaScript pattern of equating nonexistent properties to undefined on access. I think hasattr() is a better way to implement this logic here, rather than client_id = self.client_id followed by a branch on if(client_id):. A general recommendation would be to never allow the application to represent a state where this attribute doesn't exist on the class.

Experimental remedy
Removing the offending use of SIGHUP from the library code, the example appears to run as expected. However, it doesn't listen to CTRL-C or other keyboard interrupts (not the best behaviour, I had plane for that terminal session :D), and it will likely not receive the expected signal if I close the terminal now. (Windows Terminal, for that matter)

[thygrrr]

Long Term Remedy

For reference, signal code differentiates between these (comments mine); and none of the Windows signals are handled; and thus the application becomes unstoppable on Windows. Thus, appropriate and portable code should be written. (I'll see if I find the time for a PR if you accept code)

I don't know what the correct signal for CTRL-C is, but CTRL-BREAK works by handling SIGBREAK on Windows.

class Signals(IntEnum): # Universal signals
    SIGABRT: int
    SIGEMT: int
    SIGFPE: int
    SIGILL: int
    SIGINFO: int
    SIGINT: int
    SIGSEGV: int
    SIGTERM: int
    
    if sys.platform == "win32": # Small and different set of signals for Windows
        SIGBREAK: int
        CTRL_C_EVENT: int
        CTRL_BREAK_EVENT: int
    else: # Unixes get the full list
        SIGALRM: int
        SIGBUS: int
        SIGCHLD: int
        SIGCONT: int
        SIGHUP: int
        SIGIO: int
        SIGIOT: int
        SIGKILL: int
        SIGPIPE: int
        SIGPROF: int
        SIGQUIT: int
        SIGSTOP: int
        SIGSYS: int
        SIGTRAP: int
        SIGTSTP: int
        SIGTTIN: int
        SIGTTOU: int
        SIGURG: int
        SIGUSR1: int
        SIGUSR2: int
        SIGVTALRM: int
        SIGWINCH: int
        SIGXCPU: int
        SIGXFSZ: int
        if sys.platform != "darwin":  #... with a small set of exceptions for MacOS
            SIGCLD: int
            SIGPOLL: int
            SIGPWR: int
            SIGRTMAX: int
            SIGRTMIN: int

[eduardoafd]

Hi @thygrrr have you managed to solve this issue? Could you provided us with some explanation here?

[eduardoafd]

To anyone looking for a quick fix, just comment out the lines that refer to SIGHUP, seems to not cause any issues, other than related to keyboard interrupts.