fix display of error messages on early shutdown (#6719)

fix #6707
fix #6716

.gitignore

@@ -2,6 +2,7 @@
 MANIFEST
 **/tmp
 /venv*
+/.venv*
 *.py[cdo]
 *.swp
 *.swo

CHANGELOG.md

@@ -7,6 +7,8 @@
 
 ## Unreleased: mitmproxy next
 
+* Fix a bug where errors during startup would not be displayed when running mitmproxy.
+  ([#6719](https://github.com/mitmproxy/mitmproxy/pull/6719), @mhils)
 * Use newer cryptography APIs to avoid CryptographyDeprecationWarnings.
   This bumps the minimum required version to cryptography 42.0.
   ([#6718](https://github.com/mitmproxy/mitmproxy/pull/6718), @mhils)

mitmproxy/addons/errorcheck.py

@@ -3,6 +3,8 @@ import logging
 import sys
 
 from mitmproxy import log
+from mitmproxy.contrib import click as miniclick
+from mitmproxy.utils import vt_codes
 
 
 class ErrorCheck:
@@ -29,8 +31,13 @@ class ErrorCheck:
         if self.logger.has_errored:
             plural = "s" if len(self.logger.has_errored) > 1 else ""
             if self.repeat_errors_on_stderr:
-                msg = "\n".join(self.logger.format(r) for r in self.logger.has_errored)
-                print(f"Error{plural} logged during startup:\n{msg}", file=sys.stderr)
+                message = f"Error{plural} logged during startup:"
+                if vt_codes.ensure_supported(sys.stderr):  # pragma: no cover
+                    message = miniclick.style(message, fg="red")
+                details = "\n".join(
+                    self.logger.format(r) for r in self.logger.has_errored
+                )
+                print(f"{message}\n{details}", file=sys.stderr)
             else:
                 print(
                     f"Error{plural} logged during startup, exiting...", file=sys.stderr

mitmproxy/master.py

@@ -58,6 +58,7 @@ class Master:
         ):
             self.should_exit.clear()
 
+            # Can we exit before even bringing up servers?
             if ec := self.addons.get("errorcheck"):
                 await ec.shutdown_if_errored()
             if ps := self.addons.get("proxyserver"):
@@ -69,14 +70,23 @@ class Master:
                     ],
                     return_when=asyncio.FIRST_COMPLETED,
                 )
-            await self.running()
-            if ec := self.addons.get("errorcheck"):
-                await ec.shutdown_if_errored()
-                ec.finish()
+                if self.should_exit.is_set():
+                    return
+                # Did bringing up servers fail?
+                if ec := self.addons.get("errorcheck"):
+                    await ec.shutdown_if_errored()
+
             try:
+                await self.running()
+                # Any errors in the final part of startup?
+                if ec := self.addons.get("errorcheck"):
+                    await ec.shutdown_if_errored()
+                    ec.finish()
+
                 await self.should_exit.wait()
             finally:
-                # .wait might be cancelled (e.g. by sys.exit)
+                # if running() was called, we also always want to call done().
+                # .wait might be cancelled (e.g. by sys.exit), so  this needs to be in a finally block.
                 await self.done()
 
     def shutdown(self):

test/mitmproxy/addons/test_errorcheck.py

@@ -6,10 +6,11 @@ from mitmproxy.addons.errorcheck import ErrorCheck
 from mitmproxy.tools import main
 
 
-def test_errorcheck(tdata, capsys):
+@pytest.mark.parametrize("run_main", [main.mitmdump, main.mitmproxy])
+def test_errorcheck(tdata, capsys, run_main):
     """Integration test: Make sure that we catch errors on startup an exit."""
     with pytest.raises(SystemExit):
-        main.mitmdump(
+        run_main(
             [
                 "-n",
                 "-s",