introduce specific exit codes for each fatal branch (#5696)#8212
Open
ChrisJr404 wants to merge 2 commits into
Open
introduce specific exit codes for each fatal branch (#5696)#8212ChrisJr404 wants to merge 2 commits into
ChrisJr404 wants to merge 2 commits into
Conversation
Every fatal path in mitmproxy / mitmdump / mitmweb funnelled through sys.exit(1), which means a parent process (CI, supervisor, watcher script) couldn't tell apart "you typed --invalid-opt" from "the file we're saving flows to became unwritable mid-stream". Two prior PRs attempted this fix (mitmproxy#5133 and mitmproxy#5882) but were both closed when the contributors went inactive. Add mitmproxy/utils/exit_codes.py with categorised constants: 10-19 startup-time problems (STARTUP_ERROR) 20-29 environment problems (NO_UTF_CONSOLE, NO_TTY) 30-39 user-supplied input (INVALID_ARGS, INVALID_OPTIONS) 40-49 I/O problems (CANNOT_PRINT, CANNOT_WRITE_TO_FILE) 50-59 debugging hooks (DEBUG_EXIT) Categories are spaced by tens so future codes can slot into the appropriate group without renumbering existing ones, mirroring the design @Prinzhorn proposed in mitmproxy#5133. Wire each existing sys.exit(1) call in: mitmproxy/addons/errorcheck.py -> STARTUP_ERROR mitmproxy/addons/save.py -> CANNOT_WRITE_TO_FILE mitmproxy/addons/termlog.py -> CANNOT_PRINT mitmproxy/tools/console/master.py -> NO_TTY mitmproxy/tools/main.py -> INVALID_ARGS, INVALID_OPTIONS mitmproxy/utils/debug.py -> DEBUG_EXIT (x2) Existing test_termlog.py::test_cannot_print updated to use the new constant. New test/mitmproxy/utils/test_exit_codes.py pins the codes for termlog (cannot print), save (cannot write), and errorcheck (startup error), plus a sanity check that all codes are distinct.
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.
Closes #5696. (Two prior attempts at this — #5133 and #5882 — were closed when their contributors went inactive. Picking it back up.)
Background
Every fatal path in mitmproxy / mitmdump / mitmweb funnels through
sys.exit(1). A parent process (CI runner, supervisor, watcher script) only sees the integer code, so it can't distinguish--invalid-opt" from…all three return
1. Real users have asked for this since #4669 in 2021.Change
Add
mitmproxy/utils/exit_codes.pywith categorised constants. Categories are spaced by tens so future additions can slot into the appropriate group without renumbering existing ones (matches the design @Prinzhorn proposed in #5133):Then thread the constants through every existing
sys.exit(1)call site:mitmproxy/addons/errorcheck.pySTARTUP_ERRORmitmproxy/addons/save.pyCANNOT_WRITE_TO_FILEmitmproxy/addons/termlog.pyCANNOT_PRINTmitmproxy/tools/console/master.pyNO_TTYmitmproxy/tools/main.py(argparse)INVALID_ARGSmitmproxy/tools/main.py(OptionsError)INVALID_OPTIONSmitmproxy/utils/debug.py(×2MITMPROXY_DEBUG_EXIT)DEBUG_EXITThe commented-out experimental
# sys.exit(1)for the non-UTF-8 console branch inconsole/master.py:221is left untouched — that's an active "let's see if not exiting affects users" experiment from 2022, not a fatal path that needs categorisation.Tests
test/mitmproxy/utils/test_exit_codes.py(new) pins the codes for termlog (cannot print), save (cannot write), and errorcheck (startup error), plus a sanity check that all codes are distinct.test/mitmproxy/addons/test_termlog.py::test_cannot_printalready asserted onexit_code == 1; updated to use the newexit_codes.CANNOT_PRINTconstant so the existing regression coverage now pins the right code.Notes
+185 / -9across 10 files (one new module + one new test file + small edits at each call site).exit_codesmodule — existing callers that readSystemExit.codefrom a subprocess just see a different integer for branches they previously couldn't disambiguate.DEBUG_EXIT = 50is the only addition (the prior PRs didn't categorise theMITMPROXY_DEBUG_EXITenv-var path).DEBUG_EXITand revertutils/debug.pytosys.exit(1)if you'd prefer keeping the debug-only path uncategorised — let me know.