Skip to content

python: replace bare except clauses with specific exception types#5074

Draft
andamasov wants to merge 2 commits intocurrentfrom
fix/bare-except-clauses
Draft

python: replace bare except clauses with specific exception types#5074
andamasov wants to merge 2 commits intocurrentfrom
fix/bare-except-clauses

Conversation

@andamasov
Copy link
Copy Markdown
Member

Summary

  • Replace all bare except: clauses with specific exception types across 5 files
  • Bare except: catches all exceptions including KeyboardInterrupt and SystemExit, which can mask critical errors and prevent proper program termination

Changes

File Exception Type Rationale
src/conf_mode/interfaces_ethernet.py:162 Exception Interface MTU lookup may fail for various OS-level reasons
src/conf_mode/vpp_interfaces_bonding.py:181 ValueError assert_mac() raises ValueError on invalid MAC
src/conf_mode/container.py:180 IndexError List comprehension [0] on empty filtered list
src/conf_mode/container.py:186 IndexError List comprehension [0] on empty filtered list
python/vyos/config.py:124 Exception ACME cert loading can fail from file I/O, parsing, etc.
python/vyos/configverify.py:49 Exception Interface min/max MTU sysfs reads may fail
python/vyos/configverify.py:436 (TypeError, ValueError) str() conversion of DH keysize parameter

Test plan

  • Verify ethernet interface configuration still works when MTU cannot be read from adapter
  • Verify VPP bonding rejects invalid MAC addresses with proper error message
  • Verify container network address validation catches missing IPv4/IPv6 prefixes
  • Verify ACME certificate loading gracefully handles missing/corrupt certificates
  • Verify MTU validation falls back to defaults when sysfs is unavailable
  • Verify Diffie-Hellman key length verification handles invalid input

🤖 Generated with Claude Code

Bare except clauses catch all exceptions including KeyboardInterrupt
and SystemExit, which can mask critical errors and prevent proper
program termination. Replace each bare except with the appropriate
specific exception type:

- interfaces_ethernet.py: except Exception (interface MTU lookup)
- vpp_interfaces_bonding.py: except ValueError (MAC validation)
- container.py: except IndexError (prefix list indexing)
- config.py: except Exception (ACME certificate loading)
- configverify.py: except Exception (MTU min/max lookup)
- configverify.py: except (TypeError, ValueError) (DH key conversion)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 20, 2026


PR title does not match the required format

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 20, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@andamasov
Copy link
Copy Markdown
Member Author

I have read the CLA Document and I hereby sign the CLA

vyosbot added a commit to vyos/vyos-cla-signatures that referenced this pull request Mar 20, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to improve Python exception handling safety by replacing bare except: clauses with specific exception types so that KeyboardInterrupt/SystemExit aren’t accidentally swallowed during config processing.

Changes:

  • Replaced bare except: with except ValueError around MAC validation in VPP bonding verification.
  • Replaced bare except: with except IndexError in container IP prefix selection logic.
  • Replaced bare except: with except Exception (and one except (TypeError, ValueError)) in MTU/DH/acme-related helper functions.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/conf_mode/vpp_interfaces_bonding.py Narrows MAC validation exception handling to ValueError.
src/conf_mode/interfaces_ethernet.py Narrows MTU clamping exception handling to Exception.
src/conf_mode/container.py Narrows list-indexing exception handling to IndexError for prefix lookups.
python/vyos/configverify.py Narrows MTU fallback exception handling to Exception; narrows DH keysize conversion handling to (TypeError, ValueError).
python/vyos/config.py Narrows ACME cert/key loading exception handling to Exception.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- vpp_interfaces_bonding: forward actual ValueError message instead of
  hardcoded multicast error, since assert_mac raises for multiple reasons
- container: catch KeyError and TypeError alongside IndexError for prefix
  lookups, since 'prefix' key may not exist

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 434 to 437
try:
keysize = str(min_keysize)
except:
except (TypeError, ValueError):
return False
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

verify_diffie_hellman_length() currently wraps only str(min_keysize) in try/except, but the later int(min_keysize) conversion can still raise ValueError for inputs like 'abc', causing an exception instead of returning False. Also, the local variable keysize is unused. Consider validating/converting min_keysize once (e.g., to an int) inside the try/except and then using that value for comparisons.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • CLI Smoketests VPP 👍 passed
  • Config tests VPP 👍 passed
  • TPM tests 👍 passed

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

Labels

Development

Successfully merging this pull request may close these issues.

2 participants