search

Tribler / anydex-core

The Universal Decentralized Exchange
GNU Lesser General Public License v3.0
9 stars 6 forks source link

Replace bare excepts with specified excepts #33

Closed Solomon1732 closed 4 years ago

Solomon1732 commented 4 years ago

There are a number of places containing bare excepts. Some of these places can be replaced with specified excepts, and some with Exception. Since some errors like SystemExit are derived from BareException and not Exception it might lead to unexpected *behavior.

Location Error raised (not exclusive) Notes
https://github.com/Tribler/anydex-core/blob/asyncio/anydex/core/community.py#L1155 RuntimeError https://github.com/Tribler/anydex-core/blob/asyncio/anydex/core/community.py#L1277
https://github.com/Tribler/anydex-core/blob/asyncio/anydex/core/community.py#L1277 RuntimeError https://github.com/Tribler/anydex-core/blob/asyncio/anydex/core/community.py#L1277
https://github.com/Tribler/anydex-core/blob/asyncio/anydex/test/instrumentation.py#L141 None https://docs.python.org/3/library/functions.html#repr
https://github.com/Tribler/anydex-core/blob/asyncio/anydex/test/util.py#L48 None https://docs.python.org/3/library/functions.html#repr
https://github.com/Tribler/anydex-core/blob/asyncio/anydex/wallet/btc_wallet.py#L85 See notes Seems low-level enough to see which exceptions are thrown. https://github.com/Tribler/anydex-core/blob/asyncio/anydex/wallet/btc_wallet.py#L67 and https://bitcoinlib.readthedocs.io/en/latest/source/bitcoinlib.wallets.html?highlight=utxos_update#bitcoinlib.wallets.HDWallet.utxos_update
Solomon1732 commented 4 years ago

Any comment on this?

devos50 commented 4 years ago

Thanks for the poke! Currently a bit busy with our scientific article...

I agree with your summary and if you can make these changes, I'll review and accept them 👍

Our Bitcoin wallet needs some love since we rely on a rather old version of bitcoinlib. Would you by any chance be interested in doing so?

Solomon1732 commented 4 years ago

Sure, I'll PR this 😄

Our Bitcoin wallet needs some love since we rely on a rather old version of bitcoinlib. Would you by any chance be interested in doing so?

Not sure what you mean by this 😅

Solomon1732 commented 4 years ago

Regarding https://github.com/Tribler/anydex-core/blob/asyncio/anydex/wallet/btc_wallet.py#L85; looking at the links in the note area it looks like this too doesn't raise any exceptions. Please correct me if I'm wrong

Solomon1732 commented 4 years ago

Our Bitcoin wallet needs some love since we rely on a rather old version of bitcoinlib. Would you by any chance be interested in doing so?

I don't mind helping to migrate it to a newer version 😄

devos50 commented 4 years ago

Great! This is a bit of a more challenging issue I think but if you're up for the challenge you're more than welcome to try 👍 . Integration of a stable Bitcoin wallet in AnyDex (and Tribler) is something that we have been struggling with for quite some time.

Solomon1732 commented 4 years ago

I think a new issue can be opened on this then, no? Instead of discussing it here. I don't mind opening one, I simply want to avoid doing so if there's no need.

devos50 commented 4 years ago

Yes, please create a new issue for this, thanks! 👍