WinDivert.send() raises exception if LastError is nonzero

[strictlymike]

If a prior Windows API function call changes the last error value for the thread, pydivert.windivert.WinDivert raises an exception of the same value. I was originally calling an API that I noted was causing GetLastError() to return 87, and I thought it was a coincidence that WinDivert.send() was raising an exception that, when printed, read:

[Error 87] The parameter is incorrect.

But after some testing, I learned that calling SetLastError(0) after calling the (unrelated) Windows API I was using yields a successful invocation of WinDivert.send() regardless of the result of this other Windows API call.

Here is an example where I have deliberately called SetLastError(1234) within a dev branch of FakeNet-NG just prior to invoking the send() method of a WinDivert object:

03/24/18 03:12:17 PM [          Diverter] ERROR: Failed to send outbound external UDP packet
03/24/18 03:12:17 PM [          Diverter]   UDP 192.168.159.132:137->192.168.159.2:137
03/24/18 03:12:17 PM [          Diverter]   [Error 1234] No service is operating at the destination network endpoint on the remote system.

I surmise that either pydivert or WinDivert is relying on the value of GetLastError() in at least one case where it has not yet called any Windows APIs that would call SetLastError(ERROR_SUCCESS).

[ffalcinelli]

Quite strange, I could call SetLastError(0) prior to call the actual binding as a, let me say, temporary solution to this. When I'll have a bit more time to spend upon this I'd like to check if the driver itself behaves in the same way, so probably this could be redirected to the WinDivert project.

[strictlymike]

Looking more closely, I think it might be raise_on_error / wrapper within windivert_dll (see pydivert/windivert_dll/__init__.py) that makes the implicit assumption that LastError will be cleared after exiting in the success case, when I found that its may in fact depend on a previous API call.

[strictlymike]

To be clear, I think the solution you proposed (calling SetLastError(0) before calling the actual binding) is appropriate given the fact that raise_on_error is expecting the value of GetLastError() to indicate the success or failure status of this operation.