Skip to content

fix: replace 4 bare excepts with except Exception#74

Open
haosenwang1018 wants to merge 1 commit into
shaxiu:mainfrom
haosenwang1018:fix/bare-excepts
Open

fix: replace 4 bare excepts with except Exception#74
haosenwang1018 wants to merge 1 commit into
shaxiu:mainfrom
haosenwang1018:fix/bare-excepts

Conversation

@haosenwang1018
Copy link
Copy Markdown

Replace 4 bare except: with except Exception:. Bare except: catches KeyboardInterrupt and SystemExit, masking real errors.

Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from bare except: to except Exception: is a meaningful improvement, as bare excepts silently swallow SystemExit, KeyboardInterrupt, and GeneratorExit, which can make the process difficult to terminate cleanly. However, the underlying issue in most of these cases isn't just the exception type — it's that exceptions are being silenced entirely with no logging.

The trans_cookies in xianyu_utils.py is the most concerning: a malformed cookie silently continues with no indication of which cookie failed or why, making debugging cookie parsing issues unnecessarily difficult. Similarly, the except Exception: pass block in main.py around line 425 discards whatever error occurred during transaction detection without any trace in the logs.

The two except Exception blocks in json_serializer are slightly more defensible since they have fallback logic, but the second one (line 329) references e in the f-string (str(e)) from an outer except scope — worth verifying that e is actually in scope there, or this will raise a NameError at runtime in some Python versions. At minimum, these silent-catch sites should log at DEBUG or WARNING level so failures are observable in production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants