Skip to content

Fix blackd error handling: split SourceASTParseError from ASTSafetyError#5080

Open
Bahtya wants to merge 2 commits intopsf:mainfrom
Bahtya:fix/blackd-source-ast-parse-error
Open

Fix blackd error handling: split SourceASTParseError from ASTSafetyError#5080
Bahtya wants to merge 2 commits intopsf:mainfrom
Bahtya:fix/blackd-source-ast-parse-error

Conversation

@Bahtya
Copy link
Copy Markdown

@Bahtya Bahtya commented Apr 6, 2026

Summary

Per @JelleZijlstra's review feedback on #5074, this PR splits the error handling instead of treating all ASTSafetyError as 400.

Changes

  1. New exception: SourceASTParseError (src/black/parsing.py)

    • Raised when the source file cannot be parsed by ast.parse() but was accepted by Black's more lenient lib2to3 parser
    • This is a user input issue, not a Black bug
  2. Updated assert_equivalent (src/black/__init__.py)

    • Source AST parse failure → SourceASTParseError (user input issue)
    • Dest AST parse failure → ASTSafetyError (Black bug → 500)
    • Non-equivalent output → ASTSafetyError (Black bug → 500)
  3. Updated blackd (src/blackd/__init__.py)

    • SourceASTParseError → 400 Bad Request
    • ASTSafetyError → 500 Internal Server Error (unchanged, caught by generic Exception handler)

Closes

Fixes #3616

Analysis of all ASTSafetyError uses

Location Cause Error type blackd status
assert_equivalent src AST parse fail User input (lenient parser) SourceASTParseError 400
assert_equivalent dest AST parse fail Black bug ASTSafetyError 500
assert_equivalent src ≠ dst Black bug ASTSafetyError 500
check_stability_and_equivalence re-raise Depends on underlying Propagated 400 or 500

- Add SourceASTParseError exception for source file AST parse failures
  (user input issue, not a Black bug)
- Keep ASTSafetyError for Black output issues (invalid code, non-equivalent)
- In blackd, return 400 for SourceASTParseError, 500 for ASTSafetyError
- Per @JelleZijlstra review feedback on psf#5074
@Bahtya
Copy link
Copy Markdown
Author

Bahtya commented Apr 11, 2026

Hi team, just wanted to follow up on this PR. Would appreciate any feedback!

@cobaltt7
Copy link
Copy Markdown
Collaborator

cobaltt7 commented Apr 12, 2026

The code looks good to me, could you just add changelog entries (one in the Output section about the new error, and one in the Blackd section about the corrected handling) and updated tests?

@cobaltt7 cobaltt7 requested a review from JelleZijlstra April 12, 2026 19:14
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 12, 2026

diff-shades results comparing this PR (1a6e264) to main (e079b7e):

--preview style: no changes

--stable style: no changes


What is this? | Workflow run | diff-shades documentation

@JelleZijlstra
Copy link
Copy Markdown
Collaborator

Looks good, but please add a changelog entry.

@Bahtya
Copy link
Copy Markdown
Author

Bahtya commented Apr 14, 2026

Thanks @cobaltt7 @JelleZijlstra for the review! I will add the changelog entries (Output section for the new error, Blackd section for the corrected handling) and update the tests. Pushing changes shortly.

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.

(🐞) blackd bad f-string fragment causes 500 response

3 participants