Skip to content

Stop using blind exceptions - #98

Open
itsoyou wants to merge 3 commits into
mainfrom
syk/stop-using-blind-exceptions
Open

Stop using blind exceptions#98
itsoyou wants to merge 3 commits into
mainfrom
syk/stop-using-blind-exceptions

Conversation

@itsoyou

@itsoyou itsoyou commented Aug 26, 2026

Copy link
Copy Markdown
Member

SYN-96

config.py is out of scope because of its ongoing refactoring from other pr.


Note

Low Risk
Behavioral change is mainly which exceptions propagate during login; auth still fails closed by returning None, with narrower catch scopes reducing risk of hiding unexpected errors.

Overview
Replaces generic Exception raises and except Exception blocks with typed errors so configuration and OAuth login failures are easier to reason about and do not swallow unrelated bugs.

Claims validation introduces InvalidClaimsValidatorError (ValueError) for bad validator DSL definitions, and ClaimsMismatchError when JWT vs introspection claim sources disagree (via all_list_elems_are_equal_return_the_elem). token_authenticator OAuth flow now catches only ClaimsMismatchError and SynapseError for those paths instead of any exception.

HttpAuth parsing raises ValueError for unknown types and malformed input; parse_auth logs and re-raises only ValidationError, KeyError, IndexError, and ValueError. Tests expect ValueError for bad auth config.

config.py is intentionally unchanged (other PR).

Reviewed by Cursor Bugbot for commit 32f937c. Bugbot is set up for automated code reviews on this repo. Configure here.

@itsoyou
itsoyou requested a review from a team as a code owner August 26, 2026 16:00
Comment thread synapse_token_authenticator/http_auth.py Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ff296f5. Configure here.

Comment thread synapse_token_authenticator/token_authenticator.py Outdated
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.85714% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.76%. Comparing base (47a36a5) to head (32f937c).

Files with missing lines Patch % Lines
synapse_token_authenticator/token_authenticator.py 28.57% 5 Missing ⚠️
synapse_token_authenticator/claims_validator.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #98      +/-   ##
==========================================
+ Coverage   76.35%   76.76%   +0.40%     
==========================================
  Files           9        9              
  Lines         791      792       +1     
  Branches      146      146              
==========================================
+ Hits          604      608       +4     
+ Misses        131      128       -3     
  Partials       56       56              
Files with missing lines Coverage Δ
synapse_token_authenticator/http_auth.py 100.00% <100.00%> (ø)
synapse_token_authenticator/utils.py 100.00% <100.00%> (ø)
synapse_token_authenticator/claims_validator.py 77.95% <20.00%> (+1.98%) ⬆️
synapse_token_authenticator/token_authenticator.py 71.84% <28.57%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47a36a5...32f937c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@itsoyou
itsoyou force-pushed the syk/stop-using-blind-exceptions branch from 56d6d60 to 32f937c Compare August 27, 2026 10:15
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.

1 participant