Skip to content

fix: replace bare except with specific exception types#68842

Open
harshadkhetpal wants to merge 7 commits into
saltstack:masterfrom
harshadkhetpal:fix/bare-except-exception
Open

fix: replace bare except with specific exception types#68842
harshadkhetpal wants to merge 7 commits into
saltstack: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 Outdated
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

@dwoz dwoz Apr 11, 2026

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.

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

@twangboy

Copy link
Copy Markdown
Contributor

Lint failures need to be addressed. Also needs a changelog and tests

@dwoz dwoz added the test:full Run the full test suite label Jun 19, 2026
@dwoz dwoz added this to the Potassium v3009.0 milestone Jun 19, 2026
harshadkhetpal and others added 7 commits July 13, 2026 11:14
Per maintainer feedback: replace bare-except/W0702 pylint disable
comments with broad-except at all modified sites; narrow x509.py
strptime exception clauses from BaseException to ValueError.
Add changelog/68842.fixed.md per reviewer request and add unit tests
for salt/matchers/ipcidr_match.py covering address match, CIDR match,
invalid target error handling, and missing proto in grains.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants