python-socketio
python-socketio copied to clipboard
miguelgrinberg
Report which namespaces remain unconnected
Issue #822 causes clients previously able to connect to unknown server namespaces to fail. The error reported is not very informative. This PR adds the list of not-connected namespaces to the exception message.
I'm not sure this is necessary. First of all, I don't like the idea of including internal information (names of namespaces) in error messages that applications may end up presenting to users, or saving to log files. I may not have been always consistent with this, and in this case it is the invalid ones that you are making public, but in general I think it is a good idea to not share internal details in error messages.
In any case, most applications connect to a single namespace, so this isn't really a problem for those apps. And if you connect to more than one, you can try connecting to the namespaces one by one to find out which ones aren't working. I don't see a need to have this message implemented in this package and be part of applications deployed to production.
Well in my case our application stopped working. The logs and errors offered no clue. I had to resort to wireshark to figure out whether it was the server or client that was initiating the disconnect. After confirming that it was client, I had to use the debugger to catch the specific message being thrown. Only then was I able to see that it was trying to connect to a name space I hadn't asked for. If anyone, client or server, had told me what the actual problem was, it would have saved time.
If you're worried about information in exceptions, I can reformulate this so that it becomes an error log instead.
On Fri, 15 Jul 2022, 15:36 Miguel Grinberg, @.***> wrote:
I'm not sure this is necessary. First of all, I don't like the idea of including internal information (names of namespaces) in error messages that applications may end up presenting to users, or saving to log files. I may not have been always consistent with this, and in this case it is the invalid ones that you are making public, but in general I think it is a good idea to not share internal details in error messages.
In any case, most applications connect to a single namespace, so this isn't really a problem for those apps. And if you connect to more than one, you can try connecting to the namespaces one by one to find out which ones aren't working. I don't see a need to have this message implemented in this package and be part of applications deployed to production.
— Reply to this email directly, view it on GitHub https://github.com/miguelgrinberg/python-socketio/pull/962#issuecomment-1185661414, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABN3FR7LJ3YGJCTAC4IQYQDVUGAOHANCNFSM53VNWDUQ . You are receiving this because you authored the thread.Message ID: @.***>
The logs and errors offered no clue.
How so? If you enable logs (add logger=True and engineio_logger=True) to your Socket.IO constructor you see all the traffic that is exchanged. You can also look at the exchanges in the Network tab of your browser.
I had to resort to wireshark
The logs would show all the low level Socket.IO traffic, but it will be annotated with package types, so it would be much easier to analyze than a wireshark dump.
After confirming that it was client, I had to use the debugger to catch the specific message being thrown. Only then was I able to see that it was trying to connect to a name space I hadn't asked for
I don't understand this. You are making it sound like you went through a lot of effort to figure out that the problem was that the client was passing a bad namespace, yet the current error message states exactly this. Your proposed change doesn't make this any better, it just shows the name of the namespace, which as I said before, is part of the internal state of the application, so it should never be shared in error messages that are sent to the client.
I'm not using a browser. We're calling this from a standalone python application. I'm sure I could have improved logging, it didn't occur to me. Like I said, I did get a single exception, telling me that it couldn't connect a namespace. I wasn't aware that I was trying to connect more than one namespace until I actually dug down. If it had given me the name of the failing namespace in the error, I would have known immediately.
Look, I'm not pushing this, if you feel that there is somehow a security issue in mentioning the names of namespaces that the client already knows about, that's fine by me. I thougth that this was a mere oversight on your part and I wanted to be helpful and make PR to amend this in the spirit of open source dev, and to aid others that might run into this. I'm closing this because I misjudged the situation.
Cheers, and thanks for making great software.