fix: don't crash printing emoji on a legacy Windows code page (FLYTE-SDK-7M) - #1456
Open
EngHabu wants to merge 1 commit into
Open
fix: don't crash printing emoji on a legacy Windows code page (FLYTE-SDK-7M)#1456EngHabu wants to merge 1 commit into
EngHabu wants to merge 1 commit into
Conversation
…SDK-7M) `flyte start devbox` dies with UnicodeEncodeError on a Windows console running a regional code page. The success panel prints a rocket and a whale; cp936 has no room for either, so the encode fails inside Rich's legacy-Windows renderer and takes the command down after the cluster has already come up. The repo already handles this for spinners (safe_spinner, #1058). This extends the same probe to arbitrary output: safe_text() leaves text untouched whenever stdout can encode it (every UTF-8 terminal), and otherwise swaps known decorative glyphs for ASCII stand-ins and replaces anything left over with '?'. Applied at the three sites that emit non-ASCII decoration: * the devbox ready panel - the crash in Sentry * print_url's default arrow prefix - reached by `flyte run` and `flyte rerun`, so the same console would have crashed there next * the home-directory warning in `flyte run` Also passes safe_spinner() to the devbox SpinnerColumn, which was the one spinner in flyte/cli that still asked for Rich's braille default. Verified against clean main with a repro that renders each of the four sites to a strict cp936 stream: all four raise UnicodeEncodeError before this change and all four render after it. (The new tests import safe_text, so on main they fail at collection and prove nothing on their own.) fixes FLYTE-SDK-7M Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
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.
Sentry: FLYTE-SDK-7M —
UnicodeEncodeError: 'gbk' codec can't encode character '\U0001f680', release 2.6.2.Sentry: FLYTE-SDK-7S — the same crash on cp1252, release 2.6.5.
What happens
flyte start devboxfinishes bringing the cluster up, prints its success panel, and then dies:The panel prints 🚀 and 🐳. A Windows console on a regional code page (cp936 here, cp1252 in western locales) has no room for either, so the encode raises inside Rich's renderer — after the work succeeded. The user sees a crash instead of their cluster URL.
The fix
The repo already solves this for spinners:
safe_spinner()(#1058) probessys.stdout.encodingand falls back to"line"when the braille glyphs won't fit. This generalises that probe to arbitrary output.safe_text()returns its input unchanged whenever stdout can already encode it — every UTF-8 terminal, so nothing changes for almost everyone. Otherwise it swaps the decorative glyphs the CLI actually emits for ASCII stand-ins (🚀/🐳 → dropped, ➡️ →->,!, ❌ →x) and replaces anything left over with?, the same substitutionerrors="replace"would make. Rich markup is untouched, so links and colours survive.Applied at the three sites that emit non-ASCII decoration:
flyte start devbox— the crash in Sentryprint_url()default➡️prefixflyte run,flyte rerunflyte run --copy-style allfrom$HOMEprint_urlis worth calling out: it is on the mainstreamflyte runpath, so the same console would have crashed there next. The reporting user also has FLYTE-SDK-7P — their uploads fail behind a proxy — which is the only reason they had not reached it yet.Also passes
safe_spinner()to the devboxSpinnerColumn, which was the last spinner underflyte/clistill asking for Rich's braille default; cp936 cannot encode those either.Verification
The new tests import
safe_text, so on main they fail at collection and prove nothing on their own. Instead I ran a repro that renders each of the four sites to a strict cp936 stream, against cleanorigin/mainand against this branch:12 new tests in
tests/flyte/cli/test_safe_text.py, including three that drive a realrich.Consoleover a strict cp936 stream.tests/flyte/cli+tests/cli: 447 passed. ruff, mypy andcheck-docstringsclean.Left alone deliberately
flyte/remote/_action.py:648builds aSpinnerColumn()with the same braille default and would crash the same way. Fixing it meansflyte.remoteimportingflyte.cli._common, which inverts the layering; it has no Sentry evidence yet, so it is called out here rather than bundled in.Update: the cp1252 case arrived
This PR predicted "cp936 here, cp1252 in western locales". FLYTE-SDK-7S is exactly that, reported on release 2.6.5 from a different host: same
_launch_devbox_richpanel, same\U0001f680, samerich/_win32_console.write_textframe — only the code page differs, which is why Sentry grouped it separately (the codec name is part of the message). No code change needed; it is the same defect and the same fix, so it is listed here so both close together.fixes FLYTE-SDK-7M
fixes FLYTE-SDK-7S