Skip to content

Fix blackd returning 500 for invalid f-strings#5074

Closed
Bahtya wants to merge 2 commits into
psf:mainfrom
Bahtya:fix/blackd-astsafetyerror-400
Closed

Fix blackd returning 500 for invalid f-strings#5074
Bahtya wants to merge 2 commits into
psf:mainfrom
Bahtya:fix/blackd-astsafetyerror-400

Conversation

@Bahtya
Copy link
Copy Markdown
Contributor

@Bahtya Bahtya commented Apr 4, 2026

Problem

When blackd receives code with invalid f-strings (e.g. f"{}"), ast.parse() raises ASTSafetyError. This falls through to the generic except Exception handler, returning HTTP 500 (Internal Server Error).

It should return HTTP 400 (Bad Request), consistent with how InvalidInput is handled.

Solution

Add a specific except ASTSafetyError handler before the generic except Exception, returning HTTP 400.

Fixes #3616

When blackd receives code with invalid f-strings (e.g. f"{}"), ast.parse()
raises ASTSafetyError. This fell through to the generic except Exception
handler, returning HTTP 500. Add specific handler to return HTTP 400.

Fixes #3616
@JelleZijlstra
Copy link
Copy Markdown
Collaborator

This is not correct. ASTSafetyError can indicate a bug in Black, which should be a 500. The linked issue appears to be a case where Black's parser is more lenient than ast.parse. Perhaps we should split that condition off into its own exception and turn it into a 400.

Before you do this make sure you understand what each use of ASTSafetyError in Black is for and what it indicates.

@Bahtya
Copy link
Copy Markdown
Contributor Author

Bahtya commented Apr 5, 2026

Thanks for the review @JelleZijlstra, and you're absolutely right. I took the time to study all the uses of ASTSafetyError in Black:

  1. Source AST parse failure (assert_equivalent, line 1635) — user's code can't be parsed by ast.parse(), but Black's more lenient parser accepted it → user input issue → 400
  2. Dest AST parse failure (assert_equivalent, line 1646) — Black produced invalid output → Black bug → 500
  3. Source ≠ Dest (assert_equivalent, line 1656) — Black produced non-equivalent output → Black bug → 500
  4. Version mismatch (format_file_contents, line 1091) — re-raises with context → depends on underlying cause

My original PR incorrectly treats ALL of these as 400, which is wrong for cases 2 and 3.

I agree with your suggestion to split the source-parse-failure case into its own exception. I'll prepare a new version that:

  1. Introduces a new exception (e.g. SourceASTParseError) for case 1
  2. Keeps ASTSafetyError for cases 2-3 (Black bugs)
  3. Updates blackd to catch SourceASTParseError → 400, while ASTSafetyError continues to fall through to the generic 500 handler

Let me know if you'd prefer a different name for the new exception, or if you'd like a different approach entirely. Happy to implement whichever direction you prefer.

…yError

Per review feedback from @JelleZijlstra:
- Source AST parse failure → SourceASTParseError → 400 (user input issue)
- Dest AST parse failure / src≠dest → ASTSafetyError → 500 (Black bug)
- New SourceASTParseError exception in parsing.py for source parse case
- blackd catches SourceASTParseError separately for correct HTTP status
@Bahtya Bahtya closed this by deleting the head repository Apr 6, 2026
Bahtya added a commit to Bahtya/black that referenced this pull request Apr 6, 2026
- 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
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

2 participants