Fix : error handling for malformed TOML files #436
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds error handling for malformed TOML files in the TagBot action. The changes ensure that parsing failures in Project.toml, Registry.toml, and Package.toml files are caught and handled gracefully with informative error messages.
Key Changes
- Added try-except blocks around TOML parsing operations in three methods:
_project,_registry_path, and_registry_url - Added test cases for malformed Project.toml and Package.toml to verify error handling behavior
- Introduced defensive programming with
.get()for accessing registry packages dictionary
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tagbot/action/repo.py | Added error handling with try-except blocks around TOML parsing in _project, _registry_path, and _registry_url methods; uses broad Exception catching to provide detailed error messages |
| test/action/test_repo.py | Added two new test cases (test_project_malformed_toml and test_registry_url_malformed_toml) to verify that malformed TOML files raise appropriate InvalidProject exceptions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
- Replace broad Exception clauses with specific exception types - _registry_path: Catch toml.TomlDecodeError and (UnicodeDecodeError, OSError) - Allows other errors (network, file I/O, programming bugs) to propagate - _registry_url: Catch toml.TomlDecodeError and UnicodeDecodeError separately - Each exception type gets a specific error message - _project: Already catches specific toml.TomlDecodeError (no change needed) This prevents masking of legitimate errors such as: - Network errors (when fetching from GitHub) - File I/O errors (missing or inaccessible files) - Encoding issues (corrupted or invalid character encodings) - Programming bugs in the codebase All 43 tests pass.
- New test: test_registry_path_malformed_toml - Verifies that malformed Registry.toml returns None - Verifies that appropriate warning is logged with error details - Follows established pattern from test_project_malformed_toml and test_registry_url_malformed_toml - Completes test coverage for all TOML parsing error cases in repo.py All 44 tests pass.
…ength limit - Break warning message into two lines (88 char limit) - Applied black formatter for consistency - All tests pass, no linting violations
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
- New test: test_registry_url_invalid_encoding - Tests handling of invalid UTF-8 bytes in Package.toml - Verifies InvalidProject exception is raised with encoding error message - Completes test coverage for all error paths in _registry_url exception handling All 45 tests pass.
- New test: test_registry_path_invalid_encoding Tests UnicodeDecodeError when Registry.toml has invalid UTF-8 bytes Verifies warning is logged with error type and message - New test: test_registry_path_file_not_found Tests OSError when Registry.toml file doesn't exist (local clone path) Verifies warning is logged with error type and message - Completes test coverage for all error paths in _registry_path exception handling - All error cases now have dedicated test cases: * TomlDecodeError (malformed TOML syntax) * UnicodeDecodeError (invalid encoding) * OSError (missing/inaccessible files) All 47 tests pass.
Applied black formatter to ensure consistent code style compliance.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- New test: test_registry_path_missing_packages_key - Valid TOML without 'packages' key returns None and logs warning - Completes coverage for _registry_path warning paths (malformed, bad encoding, missing file, missing key) All 48 tests pass.
- New test: test_registry_url_missing_repo_key - Valid TOML without 'repo' key now raises InvalidProject as expected - Completes error-path coverage for _registry_url (malformed, encoding error, missing repo key) All 49 tests pass.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
- _project now raises InvalidProject on encoding errors - New test: test_project_invalid_encoding - Completes error-path coverage for _project (malformed TOML, encoding) All 50 tests pass.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@IanButterworth fixed/added all the copilot suggestions. |
- Update test_project_invalid_encoding to match 'Failed to parse Project.toml' - Consistent with test_project_malformed_toml pattern - More robust matching that covers both malformed TOML and encoding errors
- test_registry_path_invalid_encoding: expect 'Failed to parse' not 'Failed to load' - test_registry_path_file_not_found: expect 'Failed to parse' not 'Failed to load' - Matches implementation error message format All 50 tests pass.
- Update match pattern to 'Failed to parse Package.toml' - Consistent with test_registry_url_malformed_toml pattern - More robust matching across both error types
1 participant
Fixes #251