secdev
sendrecv silently catching OSErrors by default
The behaviour of sndrcv methods has changed quite a bit lately (due to the change to AsyncSniffers). Currently, AsyncSniffer's _run() method catches all Exceptions (in case of OSErrors they are not even logged, since they are listed as "allowed exceptions").
Most of our scripts rely on being able to catch these Exceptions in order to detect socket issues. Also I think it is generally very unintuitive that these Exceptions are caught when using a socket's sr methods, where the sniffer is running synchronously. What is your take on Exception handling within scapy?
A workaround in the current master branch is e.g. setting SuperSocket's read_allowed_exceptions to some dummy Exception and setting conf.debug_dissector to >= 2.
setting SuperSocket's read_allowed_exceptions to some dummy Exception
This was supposed to do it, but it seems I left a placeholder instead of an actual dummy exception.
https://github.com/secdev/scapy/blob/f9311c2d2c65f09eedede08a347ee541f32c132e/scapy/sendrecv.py#L903
in case of OSErrors they are not even logged, since they are listed as "allowed exceptions"
That's surprising.
Also I think it is generally very unintuitive that these Exceptions are caught when using a socket's sr methods, where the sniffer is running synchronously.
sniff is catching the exceptions because it is supposed to be able to support sniffing on multiple sockets at the same time. Whenever one crashes, it will discard it and continue to sniff on the others until either there are none left, either a stop condition is reached.
However, I agree that this isn't intuitive AND should be documented.
Although, I think setting conf.debug_dissector to 2 would be the expected behavior. Setting it to 1 displays packet dissection failures (which are hidden by default), and 2 raises exceptions.
What do you think ?
From the name conf.debug_dissector i would not have guessed that this setting is used to change scapy's exception handling. Maybe a separate variable or better naming would help here.
Other than that, i think this solution is acceptable, provided that read_allowed_exceptions is actually set to a dummy exception by default.