Skip to content

fix: replace 5 bare except clauses with except Exception#2072

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

fix: replace 5 bare except clauses with except Exception#2072
haosenwang1018 wants to merge 1 commit intoQwenLM:mainfrom
haosenwang1018:fix/bare-excepts

Conversation

@haosenwang1018
Copy link
Copy Markdown

What

Replace 5 bare except: clauses with except Exception:.

Why

Bare except: catches BaseException, including KeyboardInterrupt and SystemExit, which can prevent clean process shutdown and mask critical errors. Using except Exception: catches all application-level errors while allowing system-level exceptions to propagate correctly.

Bare `except:` catches BaseException including KeyboardInterrupt and
SystemExit. Replaced 5 instances with `except Exception:`.
Copy link
Copy Markdown

@guicybercode guicybercode left a comment

Choose a reason for hiding this comment

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

This is correct and straightforward. Replacing bare except: with except Exception: is the right call — KeyboardInterrupt and SystemExit inherit from BaseException, not Exception, so they'll propagate properly now. No behavior change for actual application errors.

Two minor observations:

In evaluate_gsm8k.py, the try block calls eval(last_number) on user-derived input. The except Exception: change is fine, but eval() on regex-extracted strings is a latent code injection risk. Out of scope for this PR, but worth a follow-up issue — int(last_number) would be sufficient here since the regex already guarantees \d+.

In evaluate_plugin.py, the commented-out # print("JSON Load Error:", action_input) silently swallows the failure and returns an empty string. Now that you're touching this line anyway, consider at least logging the exception with logging.debug or uncommenting that print. Silent failures in JSON parsing are painful to debug. Again, out of scope, but since you're already here.

TL;DR: Ship it. Clean mechanical fix, no functional risk, improves process signal handling across the board.

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