Skip to content

Fix UnicodeDecodeError reading non-UTF-8 .py module files - #281

Merged
mikeysklar merged 2 commits into
adafruit:mainfrom
cpruijsen:fix/issue-222
Sep 15, 2026
Merged

mikeysklar merged 2 commits into
adafruit:mainfrom
cpruijsen:fix/issue-222

Conversation

@cpruijsen

@cpruijsen cpruijsen commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

extract_metadata in circup/shared.py opened .py files as UTF-8 and let the decode error
propagate, so a single file saved in a Windows ANSI encoding, one with curly quotes in a comment for
instance, crashed the whole scan with UnicodeDecodeError rather than being skipped or read.

This opens those files with errors="replace". The dunder regex only reads the ASCII parts, so
replacing undecodable bytes is enough for cp1252, latin-1 and mac-roman alike, and a file that
previously crashed now yields its __version__ and __repo__.

Files that already decoded as UTF-8 are unaffected.

The new test writes a cp1252 encoded file with a curly quote and asserts the metadata comes back
rather than an exception. The existing test_extract_metadata_python asserts the open() call
signature, so it moves to the new one.

Fixes #222

Fall back to cp1252 (Windows ANSI) when a .py file is not valid UTF-8
so that metadata extraction does not crash circup. Fixes adafruit#222.
@tannewt
tannewt requested a review from a team September 14, 2026 22:20
@mikeysklar

mikeysklar commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Would open(path, encoding="utf-8", errors="replace") alone do it? Also, was this AI-assisted? We ask that you mention AI usage when submitting PRs.

Comment thread circup/shared.py Outdated
@mikeysklar

Copy link
Copy Markdown
Contributor

I tried the errors="replace" version against a real board (Metro RP2350, CircuitPython 10.4.0-alpha.1) with a cp1252 library on it. list, freeze, install and update all worked, and the board still imported the library fine before and after.

The dunder regex only reads ASCII, so replacing undecodable bytes covers
cp1252, latin-1 and mac-roman alike and drops the second open() and the
extra pylint disable.

The existing extract_metadata test asserts the open() call signature, so
it moves to the new one.
@cpruijsen

Copy link
Copy Markdown
Contributor Author

Done in a498363. One thing your suggestion could not see: test_extract_metadata_python asserts the open() call signature, so that assertion moved to include errors="replace". PR description updated to match.

On AI usage: yes, this was AI-assisted. I draft and run the tests with Claude, and review and verify everything before it goes up. Happy to record that in the PR description if you would rather have it there.

@mikeysklar mikeysklar left a comment

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.

Thanks! No need to change this one, but please note AI usage in the PR description going forward.

@mikeysklar
mikeysklar merged commit 56987ae into adafruit:main Sep 15, 2026
1 check passed
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.

UnicodeDecodeError when reading ANSI files

2 participants