Skip to content

🐛 Read email templates as UTF-8 to fix UnicodeDecodeError on Windows - #2512

Closed
AbdullahAljelamneh wants to merge 2 commits into
fastapi:masterfrom
AbdullahAljelamneh:fix-email-template-encoding
Closed

AbdullahAljelamneh wants to merge 2 commits into
fastapi:masterfrom
AbdullahAljelamneh:fix-email-template-encoding

Conversation

@AbdullahAljelamneh

Copy link
Copy Markdown

Pull Request

Discussion:

Description

AI Disclaimer

AI transcript

Checklist

  • This PR links to a GitHub Discussion for the proposed code change.
  • I added tests for the change.
  • The new or updated tests fail on the main branch and pass on this PR.
  • Coverage stays at 100%.
  • The documentation explains the change if needed.

@zyzkon

zyzkon commented Oct 29, 2026

Copy link
Copy Markdown

Confirming this fix works — reproduced independently on a different machine, and it resolves the issue.

Environment

  • Windows, system default code page 936 (GBK), PowerShell
  • Python 3.14.10, FastAPI 0.141.1
  • Fresh clone of master, no unrelated changes

Before the fix

POST /api/v1/password-recovery/[email protected] → 500 Internal Server Error

UnicodeDecodeError: 'gbk' codec can't decode byte 0x80 in position 658: illegal multibyte sequence
  File "backend/app/utils.py", line 28, in render_email_template
    ).read_text()

After applying encoding="utf-8"

Same request → 200, and the recovery email is delivered to Mailpit correctly.

Two things that may help this get merged:

  1. TRAINING.md looks accidentally committed. It's a 125-line personal study log unrelated to the fix, and it's what makes this PR hard to review. Dropping it leaves a clean one-line diff that maintainers can merge immediately.
  2. The repo asks for a linked GitHub Discussion before opening a PR (see the PR template). Adding that link should unblock review.

One more note for the fix itself: render_email_template() is the only Path.read_text() call in the backend without an explicit encoding, so this single change covers all three templates (test_email.html, reset_password.html, new_account.html).

Regarding the missing regression test: this bug does not reproduce on UTF-8 CI runners, so a portable test is difficult. I'd suggest that shouldn't block the merge.

For release notes, this would fall under bug (the latest-changes/label check is still pending).

@github-actions

Copy link
Copy Markdown
Contributor

This was marked as potentially AI generated and will be closed now. If this is an error, please provide additional details, make sure to read the docs about contributing and AI.

@github-actions github-actions Bot closed this Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants