Skip to content

fix: keep the startup banners from crashing on a non-UTF-8 stdout - #3050

Merged
soxoj merged 1 commit into
soxoj:mainfrom
ntdatt812:fix/banner-encoding-crash
Aug 31, 2026
Merged

soxoj merged 1 commit into
soxoj:mainfrom
ntdatt812:fix/banner-encoding-crash

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

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:

UnicodeEncodeError: 'charmap' codec can't encode character '♥'
  in position 1: character maps to <undefined>

_format_donate_banner puts ♥ (U+2665) in the text, and print() encodes with the codepage Python took for stdout. Reproduced on current main (5059b4b):

PYTHONIOENCODING=cp1252   returncode=1   UnicodeEncodeError: '♥'
PYTHONIOENCODING=utf-8    returncode=0

Two things make it worse than it first looks:

  • --no-color does 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.
  • PYTHONIOENCODING cannot rescue the packaged build. maigret_standalone.exe is a PyInstaller bundle, and PyInstaller strips PYTHON* variables from the child environment. chcp 65001 does 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_encodable catches 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 from QueryNotifyPrint.__init__ — replaces sys.stdout with 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_banner also 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_them runs 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 by test_debug_response_logging_writes_non_ascii_page.

Mutation-checked. Restoring the plain print() on the donate banner:

FAILED tests/test_notify.py::test_banners_survive_a_stdout_that_cannot_encode_them
1 failed, 14 deselected

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 on main.
  • flake8 warning pass reports E401 at tests/test_notify.py:134 and E303 at :182. Both reproduce identically on unmodified main — my test starts at line 209.
  • I did not run make format. black --check (26.5.1, inside the pinned >=25.1,<27.0) already fails on unmodified maigret/notify.py and tests/test_notify.py, and the reformatting it wants is in code I did not touch. It also wants to restyle the existing textwrap.dedent( calls in tests/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:37 uses a braille spinner and checking.py uses → 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.

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.
@soxoj
soxoj merged commit 3df91b4 into soxoj:main Aug 31, 2026
6 checks passed
@soxoj

soxoj commented Aug 31, 2026

Copy link
Copy Markdown
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).
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.

2 participants