Repository navigation
fix: keep the startup banners from crashing on a non-UTF-8 stdout - #3050
Merged
Merged
Conversation
The intro and donate banners carry U+2665, which the Windows ANSI codepage cannot encode — cp1252 on a default install — so maigret raised UnicodeEncodeError from print() before a single site was checked. --no-color did not help: the character is in that branch of the formatter too, and the PyInstaller build ignores PYTHONIOENCODING, so there was no way around it from outside the process either. The banners now fall back to an encodable rendering instead of raising. The write is caught rather than the stream reconfigured because colorama's init() replaces sys.stdout with a wrapper that has no reconfigure(), so a stream-level fix would depend on running before it.
Owner
|
@ntdatt812 great first PR, thanks! 👍 Appreciate that you mutation-checked your own test instead of trusting it. And you listed the other places with the same problem rather than quietly folding them in. I reran the mutation, it fails exactly as you said. I filed those other places as #3055 if you want them. 🚀 |
soxoj
pushed a commit
that referenced
this pull request
Aug 31, 2026
…out (#3056) * fix: keep result lines from crashing on a non-UTF-8 stdout #3050 stopped the startup banners from raising UnicodeEncodeError on a Windows console, but the banner was not the only line the ANSI codepage can break. Two ordinary things a run prints still went through a bare print(): - the --enrich notifications built in checking.py carry U+2192, which cp1252 has no slot for (checking.py:1847, 1851, 1859, 1899); - any profile field maigret extracts carries the script the page was written in, so a Cyrillic or CJK fullname breaks the result line that reports it. Both arrive mid-scan, after the network work is done, so the crash discarded results the run had already collected -- worse than failing at startup. Route every write in notify.py through the existing _print_encodable helper. Verified on a real cp1252 stdout, not a mock: both cases raise before this change and print with replacement characters after it. * fix: keep the AI spinner alive on a stderr that cannot encode braille Second half of #3055. The spinner frames are braille, which cp1252 -- the ANSI codepage of a stock Windows install -- cannot encode, so the first frame raised UnicodeEncodeError inside the daemon thread: the thread died with a traceback printed over the output and the animation stopped for the rest of the run. Encoding with replacement would only leave a row of '?' spinning, so pick the frame set from what the stream can carry instead. A UTF-8 terminal keeps the braille; anything that cannot hold it gets |/-\ and still animates. print_streaming() in the same module writes model output the same way and would raise on any non-Latin reply, but it has no callers anywhere in the tree, so it is left alone rather than fixed as dead code.
ntdatt812
added a commit
to ntdatt812/maigret
that referenced
this pull request
Sep 7, 2026
Third of the same shape as soxoj#3050 (startup banners) and soxoj#3056 (result lines and the spinner). The one terminal write in the AI module that was not covered is the analysis itself. _stream_response() writes each token of the model's reply straight to stdout. The reply is arbitrary text, and an OSINT run on an international username routinely comes back with Cyrillic or CJK. On Windows, Python takes stdout's encoding from the process ANSI codepage -- cp1252 on a default install -- so that write raises UnicodeEncodeError, the exception leaves the coroutine, and the analysis ends mid-sentence with the tokens already printed left on screen. sys.stdout's error handler is surrogateescape, which is not a guard here: it rescues lone surrogates only, so an ordinary unencodable character still raises. There is a test that pins exactly that, so this suite notices if the platform ever changes. Also corrects the _frames_for docstring, which I wrote in soxoj#3056. It says the braille frames "raised UnicodeEncodeError inside the spinner's daemon thread". They did not: unlike stdout, stderr's error handler is backslashreplace, so the write never raised -- it printed the escape, and the animation became a column of literal ⠋. The fix was right, the reason given for it was not, and the difference between the two streams is exactly what this change turns on. print_streaming() writes to stdout the same way and has the same defect, but it has no callers anywhere in the tree, so it is left alone rather than changed without a way to exercise it. Verified: 6 tests added to tests/test_ai.py, driven by real cp1252 streams rather than mocks so the codec decides. Three mutations, all killed. Full suite on Windows: the set of failing test names is identical before and after (35 pre-existing).
soxoj
pushed a commit
that referenced
this pull request
Oct 10, 2026
Third of the same shape as #3050 (startup banners) and #3056 (result lines and the spinner). The one terminal write in the AI module that was not covered is the analysis itself. _stream_response() writes each token of the model's reply straight to stdout. The reply is arbitrary text, and an OSINT run on an international username routinely comes back with Cyrillic or CJK. On Windows, Python takes stdout's encoding from the process ANSI codepage -- cp1252 on a default install -- so that write raises UnicodeEncodeError, the exception leaves the coroutine, and the analysis ends mid-sentence with the tokens already printed left on screen. sys.stdout's error handler is surrogateescape, which is not a guard here: it rescues lone surrogates only, so an ordinary unencodable character still raises. There is a test that pins exactly that, so this suite notices if the platform ever changes. Also corrects the _frames_for docstring, which I wrote in #3056. It says the braille frames "raised UnicodeEncodeError inside the spinner's daemon thread". They did not: unlike stdout, stderr's error handler is backslashreplace, so the write never raised -- it printed the escape, and the animation became a column of literal ⠋. The fix was right, the reason given for it was not, and the difference between the two streams is exactly what this change turns on. print_streaming() writes to stdout the same way and has the same defect, but it has no callers anywhere in the tree, so it is left alone rather than changed without a way to exercise it. Verified: 6 tests added to tests/test_ai.py, driven by real cp1252 streams rather than mocks so the codec decides. Three mutations, all killed. Full suite on Windows: the set of failing test names is identical before and after (35 pre-existing).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The crash
On a Windows box whose ANSI codepage is not UTF-8 — cp1252 is the default install — maigret dies before it checks a single site:
_format_donate_bannerputs♥(U+2665) in the text, andprint()encodes with the codepage Python took for stdout. Reproduced on currentmain(5059b4b):Two things make it worse than it first looks:
--no-colordoes not help. The character is in both branches of the formatter, the plain one included, so the obvious workaround for a terminal problem does nothing.PYTHONIOENCODINGcannot rescue the packaged build.maigret_standalone.exeis a PyInstaller bundle, and PyInstaller stripsPYTHON*variables from the child environment.chcp 65001does not help either — it changes the console codepage, not the ANSI codepage Python reads. So for a user of the released binary there is no way around this from outside the process.The banner prints unconditionally at startup, so the effect is that maigret does not run at all on those machines.
The fix
_print_encodablecatches the failing write and re-renders the text in something the stream can hold, so the banner degrades instead of taking the process down.Catching the write rather than reconfiguring the stream is deliberate, and it is the part I would most like a second opinion on.
colorama.init()— called fromQueryNotifyPrint.__init__— replacessys.stdoutwith a wrapper that has no.reconfigure(), so a stream-level fix only works if it runs first, and it would then quietly stop working the moment something constructs a notifier earlier. Catching the write does not depend on ordering.Both banners go through it.
_format_donate_banneralso carries an em dash, which cp1252 happens to have but several other codepages do not, so it is covered by the same guard rather than by a second special case.Test
test_banners_survive_a_stdout_that_cannot_encode_themruns the banners in a child process whose stdout really is cp1252, rather than mocking the failure — so it fails on any machine if the guard goes, not only on a Windows one. It follows the child-process shape already used bytest_debug_response_logging_writes_non_ascii_page.Mutation-checked. Restoring the plain
print()on the donate banner:Restored:
15 passed.Notes on the lint gates
flake8 --select=E9,F63,F7,F82(the blocking pass): 0.mypy --check-untyped-defs maigret/notify.py: one error,Library stubs not installed for "colorama", which reproduces unchanged onmain.flake8warning pass reportsE401attests/test_notify.py:134andE303at:182. Both reproduce identically on unmodifiedmain— my test starts at line 209.make format.black --check(26.5.1, inside the pinned>=25.1,<27.0) already fails on unmodifiedmaigret/notify.pyandtests/test_notify.py, and the reformatting it wants is in code I did not touch. It also wants to restyle the existingtextwrap.dedent(calls intests/test_checking.py, which is the shape my test copied. Running it would have buried this change in unrelated churn, so I matched the surrounding style instead. Happy to run it if you would rather the branch be formatted with a specific version.Other sites of the same class, not touched here
ai.py:37uses a braille spinner andchecking.pyuses→in verbose messages. Both are behind flags rather than on the startup path, so they are a smaller problem than this one, and folding them in would mean routing several unrelated call sites through the new helper in the same change. Happy to follow up if you want them covered.