Skip to content

fix: replace bare except with specific exception types#68842

Open
harshadkhetpal wants to merge 5 commits intosaltstack:masterfrom
harshadkhetpal:fix/bare-except-exception
Open

fix: replace bare except with specific exception types#68842
harshadkhetpal wants to merge 5 commits intosaltstack:masterfrom
harshadkhetpal:fix/bare-except-exception

Conversation

@harshadkhetpal
Copy link
Copy Markdown

Summary

Replace bare except: clauses with specific exception types across 5 files (7 occurrences).

Bare except: catches all exceptions including SystemExit, KeyboardInterrupt, and GeneratorExit, which should typically propagate. Per PEP 8 (E722) and flake8-bugbear (B001):

  • Sites that re-raise → except BaseException:
  • Sites that swallow errors → except Exception:

Files changed:

  • salt/matchers/ipcidr_match.py — 2× except Exception:
  • salt/roster/dir.py — 1× except Exception:
  • salt/modules/x509.py — 2× except BaseException: (has raise)
  • salt/beacons/__init__.py — 1× except Exception:
  • salt/states/saltmod.py — 1× except Exception:
# Before
except:

# After (no re-raise)
except Exception:

# After (with re-raise)
except BaseException:

Testing

No behavior change — style/correctness fix only.

@harshadkhetpal harshadkhetpal requested a review from a team as a code owner March 25, 2026 03:51
Comment thread salt/beacons/__init__.py
try:
raw = self.beacons[fun_str](b_config[mod])
except: # pylint: disable=bare-except
except Exception: # pylint: disable=bare-except
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also remove # pylint: disable=bare-except if removing the bare except.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

except Exception is essentially just a bare except

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It isn't a bare expect it is a broad expect so I think the correct disable would be # pylint: disable=broad-except

Copy link
Copy Markdown
Contributor

@dwoz dwoz Apr 11, 2026

Choose a reason for hiding this comment

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

Okay, I hadn't properly looked at this when I made my comment. I'm good with this change, provided we use broad-except for pylint.

@harshadkhetpal

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.

3 participants