search

polkascan / py-substrate-interface

Python Substrate Interface
https://polkascan.github.io/py-substrate-interface/
Apache License 2.0
239 stars 111 forks source link

Unhandled WebSocketConnectionClosedException #383

Open h4nsu opened 4 months ago

h4nsu commented 4 months ago

Occasionally this line: https://github.com/polkascan/py-substrate-interface/blob/84485c1f7774016ed2f93380b3a6ec041c28b2c1/substrateinterface/base.py#L329 blows up with WebSocketConnectionClosedException. Would it be ok to wrap it with an auto-reconnect try-catch just like here: https://github.com/polkascan/py-substrate-interface/blob/84485c1f7774016ed2f93380b3a6ec041c28b2c1/substrateinterface/base.py#L271-L282

arjanz commented 4 months ago

Yes handling this in the application is the preferred way. There is an 'auto_reconnect' feature built in (auto_reconnect=True during init), which will work as well, but will hide side-effects like subscriptions being reset from the application.

h4nsu commented 4 months ago

Sorry, what I wrote was confusing. Here's what I meant:

This: https://github.com/polkascan/py-substrate-interface/blob/84485c1f7774016ed2f93380b3a6ec041c28b2c1/substrateinterface/base.py#L272 is wrapped with a try-block and handles broken sockets transparently when auto_reconnect=True.

Shouldn't the same practice be applied here: https://github.com/polkascan/py-substrate-interface/blob/84485c1f7774016ed2f93380b3a6ec041c28b2c1/substrateinterface/base.py#L329 so that I don't need to reconnect manually in my application and I can simply rely on auto_reconnect doing what it promises? ;)

arjanz commented 4 months ago

Ah I see what you mean now.. You are most likely right, now probably in case of a subscription with a long interval between responses the connection could be closed. The question is what will happen if during an active subscription the result_handler and its update_nr will be reset, I'll have to test if there are undesired side-effects.

If there aren't any I can just expand the try/catch and I'll link a PR, otherwise I think it's best to move to checks to the application, so there is more control what to do with these edge cases.