fix(tinytorch): tito crashes on Windows when a subprocess's output can't be decoded as cp1252 - #1971
Open
Shashank-Tripathi-07 wants to merge 1 commit into
Conversation
…n't be decoded as cp1252
Found by actually running through the TinyTorch student workflow end to
end on Windows: `tito module test 01` (a command every student runs
constantly to check their own work) crashes with a raw traceback:
TypeError: sequence item 1: expected str instance, NoneType found
Root cause: tito/commands/module/test.py runs pytest via
subprocess.run(..., capture_output=True, text=True) without specifying
an encoding. On Windows, text mode defaults to decoding the subprocess's
output using the console's legacy codepage (cp1252 here), not UTF-8.
Since pytest's own output contains characters cp1252 can't represent,
the background reader thread that decodes it dies with an unhandled
UnicodeDecodeError, and subprocess.run silently returns None for that
stream instead of the expected string. That None then blows up
"\n".join(all_output) in test_module(), taking the whole command down.
This is the same root cause fixed earlier in tito/main.py (the CLI's
own stdout/stderr) and in install.sh, just in code that was never
touched by those fixes: every subprocess.run/Popen call across the
tito package that captures output as text. A repo-wide check found
this exact gap in 13 files and ~30 call sites -- everywhere except two
calls in module/workflow.py that had already been fixed. Since it's one
root cause (no consistent subprocess text encoding) rather than 30
unrelated bugs, this fixes it everywhere in one pass: every
`text=True` subprocess call now also specifies
`encoding="utf-8", errors="replace"`, so output that can't be
represented gets replaced with a placeholder character instead of
crashing the reader thread.
Verified by re-running the failing command (`tito module test 01`)
after the fix: completes cleanly with no crash. Also re-ran the full
student workflow (setup, modules 01-09, milestones 01-04, module
reset, module test, community status, dev export --help) with no
other crashes found.
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.
What I did
Ran through the full TinyTorch student workflow on a real Windows machine (install -> setup -> modules 01-09 -> milestones 01-04 -> module test/reset -> community/dev commands) to hunt for the kind of fatal, no-recourse errors this project has had (see #1960, #1966/#1969).
The bug
tito module test 01-- a command every student runs constantly -- crashes outright:Root cause
tito/commands/module/test.pyruns pytest viasubprocess.run(..., capture_output=True, text=True)without anencoding=. On Windows, text mode decodes the subprocess's output using the console's legacy codepage (cp1252 here), not UTF-8. pytest's own output contains characters cp1252 can't represent, so the background thread that reads and decodes the pipe dies with an unhandledUnicodeDecodeError-- andsubprocess.runsilently gives backNonefor that stream instead of raising or returning a string. ThatNonethen blows up"\n".join(all_output), taking the whole command down with a raw traceback.This is the same root cause already fixed in
tito/main.py's own stdout/stderr (part of an earlier PR) and ininstall.sh, just in code neither of those touched. I checked the rest of thetitopackage for the same pattern and found it in 13 files, ~30 call sites -- everysubprocess.run/Popencall usingtext=Trueexcept two inmodule/workflow.pythat had already been fixed at some point. Since it's one root cause (no consistent subprocess text encoding policy), not 30 unrelated bugs, this fixes all of them in one pass: everytext=Truecall now also specifiesencoding="utf-8", errors="replace", so undecodable output gets replaced with a placeholder character instead of killing the reader thread.Files touched:
commands/dev/preflight.py,commands/dev/test.py,commands/export_utils.py,commands/milestone.py,commands/module/test.py,commands/module/workflow.py(the Jupyter-launchPopenthat hadn't been covered),commands/nbgrader.py,commands/package/nbdev.py,commands/setup.py,commands/system/health.py,commands/system/update.py,core/auth.py,core/status_analyzer.py.Testing
tito module test 01, confirmed the fix resolves it (runs clean, all phases pass).tito setup, modules 01 through 09 (start/complete), milestones 01-04 (perceptron, XOR, MLP, CNN -- including real training),module test,module reset,module status,community status,dev export --help, and a sweep of everytito <group> --helpplus deliberately invalid invocations (bad module numbers, missing arguments, unknown milestones).python -m py_compileon every changed file.