Returning DISABLE from a sys.monitoring callback can trigger an assert in a debug build in (3.12b4)

[jpe]

Returning DISABLE from a sys.monitoring callback for an event >= PY_MONITORING_INSTRUMENTED_EVENTS triggers an assert in a debug build. It would probably be better to just ignore a DISABLE return value since anything else can be returned (I think) or mandate that None be returned. If the current behavior is retained, it should be documented.

I didn't test in a non debug build.

Linked PRs

• GH-106895: Raise a ValueError when attempting to disable events that cannot be disabled. #107337
• [3.12] GH-106895: Raise a ValueError when attempting to disable events that cannot be disabled. (GH-107337) #107351

[jpe]

I mentioned this in #103082 but was encouraged to create a separate bug for it.

[gaogaotiantian]

Anything else other than DISABLE would be considered "not disable" and behave the same.

I agree that the assertion should be treated. However, the first thing is to figure out the expected behavior. For events >= PY_MONITORING_INSTRUMENTED_EVENTS, normally an exception is already set. Do we want to silently ignore the DISABLE, or send out a message to the user? Through exception or other methods? We definitely do not want to overwrite the current exception being raised, so nested exception? The current monitoring code does not even deal with errors in such case - you can return an error code and it was just ignored eventually. So maybe we don't want to raise any exceptions in monitoring code?

[jpe]

The events >= PY_MONITORING_INSTRUMENTED_EVENTS cannot be disabled per PEP 669. Note that the monitoring code is not raising an exception in this case (that's another bug) and that DISABLE is just an ordinary return value. I guess my expectation was it would just be ignored -- I have generic filtering code that was always returning DISABLE for some files. I could see maybe emitting a warning through the warning module if DISABLE was returned.

[gaogaotiantian]

I'm not sure if we want to involve warning module in this case. I can accept if DISABLE is silently ignored for events that do not support it. However, it's still a design decision so let's wait for the response from @markshannon .

[markshannon]

I don't think a warning makes sense.

I think we should raise, rather than ignore DISABLE.
By raising an exception we retain the ability to add disabling in future versions.

[gaogaotiantian]

So we want to raise an exception from the already raised exception (as the event is for dealing with exceptions)? An ValueError maybe?