fix: replace bare except with except Exception in types/base.py - #580
fix: replace bare except with except Exception in types/base.py#580KeloYuan wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens exception handling in Chunk._preview_embedding() by replacing a bare except: with except Exception:, avoiding suppression of non-standard exceptions (e.g., KeyboardInterrupt, SystemExit) during embedding preview generation used in __repr__.
Changes:
- Replace bare
except:withexcept Exception:in_preview_embedding().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request updates the exception handling in src/chonkie/types/base.py by replacing a bare except clause with except Exception. The reviewer suggests further refining this by catching specific exceptions to avoid suppressing unexpected internal errors, providing a more robust implementation in line with PEP 8 recommendations.
| else: | ||
| return str(self.embedding) | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
While replacing a bare except: with except Exception: is an improvement as it avoids catching system-exiting exceptions (like KeyboardInterrupt), it is still considered a broad catch. For a more robust implementation, it is better to catch only the specific exceptions that are expected to occur during the formatting and indexing of the embedding, such as AttributeError, TypeError, ValueError, IndexError, and KeyError. This ensures that unexpected internal errors are not silently suppressed.
| except Exception: | |
| except (AttributeError, TypeError, ValueError, IndexError, KeyError): |
References
- PEP 8 recommends catching specific exceptions whenever possible instead of using a broad except Exception: clause. (link)
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request narrows exception handling in the ChangesException Handling Specificity
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Replaces bare
except:withexcept Exception:insrc/chonkie/types/base.py(line 82).Summary by CodeRabbit