Actually, it was introduced in 2542913 (#56) and was intended to ignore AttributeError error (this is not the case after #273). The TODO to check what it actually ignores was added in 8868bd4 and then the try-except was removed in #273. (This was just a small research)

Why Exception? Why not a more specific type?

I am unsure that we care about any exceptions there. I think that it's something wrong with the user OS, but who cares, the socket is used and needs to be only closed. So if the socket failes to close, and we got all info that we need - let's just ignore that.

And ideally, though I don't insist on this, add a test ensuring this handling works as expected in this scenario.

Looking at #140, I'm unsure that mocking socket is a good idea.

Wow, the project is so big and old...

This gives me flashbacks to the reason I started maintaining mcstatus. The first PR I merged looked very similar to this.

I am unsure that we care about any exceptions there. I think that it's something wrong with the user OS, but who cares, the socket is used and needs to be only closed. So if the socket failes to close, and we got all info that we need - let's just ignore that.

Actually it's pretty important to ensure that the OS recognizes the socket as closed, and removes that resource from the process. That's because most kernels have limitations for processes, such as a maximum amount of file descriptors, which sockets do take up (see: #337). This can then end up causing weird errors like "OSError: Too many open files", when creating a new socket connection.

Because of that, I'm still not sure it's such a good idea to just blindly ignore all exceptions here, and it's probably worth investigating why this actually occurred, and looking into it a bit more.

Errno 107 means that the socket is NOT connected (any more).

https://stackoverflow.com/a/4668710

Would it be okay to something, like here?

Yes! That looks like a much better way to handle this, with the rest of the exceptions still getting raised, but do notice that this handling is done in socket.shutdown, not in socket.close, instead, close is still called without any handling below.

So I suppose what's going on is that if some internal flag in the socket instance, representing connection status is on. When close is then called, if this flag is on, shutdown is likely getting called before the actual closing, to properly send the disconnection packets. So, I suppose that if it's done manually like this, that flag gets toggled, even if we get an exception later. So calling close afterwards is safe, and that's what then makes the syscall, telling the kernel to remove that socket from the process.

I quite like this solution, but I'm not sure if I'm correct about this internal flag, if this is the case, we can use this solution.

We shutdown socket too, just in other function.

Then the handling should be done in that function, not in the exit functions, and only on the shutdown.

When handling this on the close function itself, if this error occurs, the socket doesn't get closed, the exception is raised from the shutdown line, so we never even get to the line below calling close. The exception from shutdown is handled and ignored, and the socket remains attached to the process.

Sorry for the delay, VRChat is more interesting that I excepected