diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fe21aadd..f9fd195a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ * Console Improvements on Windows (@mhils) * Fix processing of `--set` options (#5067, @marwinxxii) * Lowercase user-added header names and emit a log message to notify the user when using HTTP/2 (#4746, @mhils) +* Exit early if there are errors on startup (#4544, @mhils) ## 28 September 2021: mitmproxy 7.0.4 diff --git a/mitmproxy/addons/errorcheck.py b/mitmproxy/addons/errorcheck.py new file mode 100644 index 000000000..b522c9e39 --- /dev/null +++ b/mitmproxy/addons/errorcheck.py @@ -0,0 +1,20 @@ +import asyncio +import sys + + +class ErrorCheck: + """Monitor startup for error log entries, and terminate immediately if there are some.""" + def __init__(self): + self.has_errored = False + + def add_log(self, e): + if e.level == "error": + self.has_errored = True + + async def running(self): + # don't run immediately, wait for all logging tasks to finish. + asyncio.create_task(self._shutdown_if_errored()) + + async def _shutdown_if_errored(self): + if self.has_errored: + sys.exit(1) diff --git a/mitmproxy/addons/proxyserver.py b/mitmproxy/addons/proxyserver.py index 06efaf974..834f31c7c 100644 --- a/mitmproxy/addons/proxyserver.py +++ b/mitmproxy/addons/proxyserver.py @@ -99,11 +99,11 @@ class Proxyserver: """, ) - def running(self): + async def running(self): self.master = ctx.master self.options = ctx.options self.is_running = True - self.configure(["listen_port"]) + await self.refresh_server() def configure(self, updated): if "stream_large_bodies" in updated: @@ -120,9 +120,7 @@ class Proxyserver: f"{ctx.options.body_size_limit}") if "mode" in updated and ctx.options.mode == "transparent": # pragma: no cover platform.init_transparent_mode() - if not self.is_running: - return - if any(x in updated for x in ["server", "listen_host", "listen_port"]): + if self.is_running and any(x in updated for x in ["server", "listen_host", "listen_port"]): asyncio.create_task(self.refresh_server()) async def refresh_server(self): @@ -133,11 +131,15 @@ class Proxyserver: if ctx.options.server: if not ctx.master.addons.get("nextlayer"): ctx.log.warn("Warning: Running proxyserver without nextlayer addon!") - self.server = await asyncio.start_server( - self.handle_connection, - self.options.listen_host, - self.options.listen_port, - ) + try: + self.server = await asyncio.start_server( + self.handle_connection, + self.options.listen_host, + self.options.listen_port, + ) + except OSError as e: + ctx.log.error(str(e)) + return addrs = {f"http://{human.format_address(s.getsockname())}" for s in self.server.sockets} ctx.log.info(f"Proxy server listening at {' and '.join(addrs)}") diff --git a/mitmproxy/hooks.py b/mitmproxy/hooks.py index 77b6d207f..6871c4b92 100644 --- a/mitmproxy/hooks.py +++ b/mitmproxy/hooks.py @@ -72,8 +72,7 @@ class DoneHook(Hook): class RunningHook(Hook): """ Called when the proxy is completely up and running. At this point, - you can expect the proxy to be bound to a port, and all addons to be - loaded. + you can expect all addons to be loaded and all options to be set. """ diff --git a/mitmproxy/tools/console/master.py b/mitmproxy/tools/console/master.py index f0f14065e..2fc286061 100644 --- a/mitmproxy/tools/console/master.py +++ b/mitmproxy/tools/console/master.py @@ -19,7 +19,7 @@ import urwid from mitmproxy import addons from mitmproxy import master from mitmproxy import log -from mitmproxy.addons import intercept +from mitmproxy.addons import errorcheck, intercept from mitmproxy.addons import eventstore from mitmproxy.addons import readfile from mitmproxy.addons import view @@ -56,6 +56,7 @@ class ConsoleMaster(master.Master): readfile.ReadFile(), consoleaddons.ConsoleAddon(self), keymap.KeymapConfig(), + errorcheck.ErrorCheck(), ) self.window = None diff --git a/mitmproxy/tools/dump.py b/mitmproxy/tools/dump.py index e2fe6f7a7..527a93e8b 100644 --- a/mitmproxy/tools/dump.py +++ b/mitmproxy/tools/dump.py @@ -1,20 +1,10 @@ from mitmproxy import addons -from mitmproxy import options from mitmproxy import master -from mitmproxy.addons import dumper, termlog, keepserving, readfile - - -class ErrorCheck: - def __init__(self): - self.has_errored = False - - def add_log(self, e): - if e.level == "error": - self.has_errored = True +from mitmproxy import options +from mitmproxy.addons import dumper, errorcheck, keepserving, readfile, termlog class DumpMaster(master.Master): - def __init__( self, options: options.Options, @@ -22,7 +12,6 @@ class DumpMaster(master.Master): with_dumper=True, ) -> None: super().__init__(options) - self.errorcheck = ErrorCheck() if with_termlog: self.addons.add(termlog.TermLog()) self.addons.add(*addons.default_addons()) @@ -31,5 +20,5 @@ class DumpMaster(master.Master): self.addons.add( keepserving.KeepServing(), readfile.ReadFileStdin(), - self.errorcheck + errorcheck.ErrorCheck(), ) diff --git a/mitmproxy/tools/main.py b/mitmproxy/tools/main.py index f9509c941..abb5e4bac 100644 --- a/mitmproxy/tools/main.py +++ b/mitmproxy/tools/main.py @@ -48,17 +48,20 @@ def process_options(parser, opts, args): opts.merge(adict) +T = typing.TypeVar("T", bound=master.Master) + + def run( - master_cls: typing.Type[master.Master], + master_cls: typing.Type[T], make_parser: typing.Callable[[options.Options], argparse.ArgumentParser], arguments: typing.Sequence[str], extra: typing.Callable[[typing.Any], dict] = None -) -> master.Master: # pragma: no cover +) -> T: # pragma: no cover """ extra: Extra argument processing callable which returns a dict of options. """ - async def main() -> master.Master: + async def main() -> T: debug.register_info_dumpers() opts = options.Options() @@ -143,9 +146,7 @@ def mitmdump(args=None) -> typing.Optional[int]: # pragma: no cover ) return {} - m = run(dump.DumpMaster, cmdline.mitmdump, args, extra) - if m and m.errorcheck.has_errored: # type: ignore - return 1 + run(dump.DumpMaster, cmdline.mitmdump, args, extra) return None diff --git a/mitmproxy/tools/web/master.py b/mitmproxy/tools/web/master.py index 201444135..b8421a0cc 100644 --- a/mitmproxy/tools/web/master.py +++ b/mitmproxy/tools/web/master.py @@ -5,7 +5,7 @@ from mitmproxy import addons from mitmproxy import log from mitmproxy import master from mitmproxy import optmanager -from mitmproxy.addons import eventstore +from mitmproxy.addons import errorcheck, eventstore from mitmproxy.addons import intercept from mitmproxy.addons import readfile from mitmproxy.addons import termlog @@ -29,6 +29,8 @@ class WebMaster(master.Master): self.options.changed.connect(self._sig_options_update) + if with_termlog: + self.addons.add(termlog.TermLog()) self.addons.add(*addons.default_addons()) self.addons.add( webaddons.WebAddon(), @@ -37,9 +39,8 @@ class WebMaster(master.Master): static_viewer.StaticViewer(), self.view, self.events, + errorcheck.ErrorCheck(), ) - if with_termlog: - self.addons.add(termlog.TermLog()) self.app = app.Application( self, self.options.web_debug ) diff --git a/test/mitmproxy/addons/test_asgiapp.py b/test/mitmproxy/addons/test_asgiapp.py index ad9cccede..d1232ce4e 100644 --- a/test/mitmproxy/addons/test_asgiapp.py +++ b/test/mitmproxy/addons/test_asgiapp.py @@ -54,7 +54,7 @@ async def test_asgi_full(): with taddons.context(ps, *addons) as tctx: tctx.master.addons.add(next_layer.NextLayer()) tctx.configure(ps, listen_host="127.0.0.1", listen_port=0) - ps.running() + await ps.running() await tctx.master.await_log("Proxy server listening", level="info") proxy_addr = ps.server.sockets[0].getsockname()[:2] diff --git a/test/mitmproxy/addons/test_errorcheck.py b/test/mitmproxy/addons/test_errorcheck.py new file mode 100644 index 000000000..e1cdd9f2a --- /dev/null +++ b/test/mitmproxy/addons/test_errorcheck.py @@ -0,0 +1,25 @@ +import asyncio + +import pytest + +from mitmproxy import log +from mitmproxy.addons.errorcheck import ErrorCheck + + +def test_errorcheck(): + async def run(): + # suppress error that task exception was not retrieved. + asyncio.get_running_loop().set_exception_handler(lambda *_: 0) + e = ErrorCheck() + e.add_log(log.LogEntry("fatal", "error")) + await e.running() + await asyncio.sleep(0) + + with pytest.raises(SystemExit): + asyncio.run(run()) + + +async def test_no_error(): + e = ErrorCheck() + await e.running() + await asyncio.sleep(0) diff --git a/test/mitmproxy/addons/test_proxyserver.py b/test/mitmproxy/addons/test_proxyserver.py index 71086a550..6581827a8 100644 --- a/test/mitmproxy/addons/test_proxyserver.py +++ b/test/mitmproxy/addons/test_proxyserver.py @@ -55,7 +55,7 @@ async def test_start_stop(): async with tcp_server(server_handler) as addr: tctx.configure(ps, listen_host="127.0.0.1", listen_port=0) assert not ps.server - ps.running() + await ps.running() await tctx.master.await_log("Proxy server listening", level="info") assert ps.server @@ -99,7 +99,7 @@ async def test_inject() -> None: tctx.master.addons.add(state) async with tcp_server(server_handler) as addr: tctx.configure(ps, listen_host="127.0.0.1", listen_port=0) - ps.running() + await ps.running() await tctx.master.await_log("Proxy server listening", level="info") proxy_addr = ps.server.sockets[0].getsockname()[:2] reader, writer = await asyncio.open_connection(*proxy_addr) @@ -154,7 +154,7 @@ async def test_warn_no_nextlayer(): ps = Proxyserver() with taddons.context(ps) as tctx: tctx.configure(ps, listen_host="127.0.0.1", listen_port=0) - ps.running() + await ps.running() await tctx.master.await_log("Proxy server listening at", level="info") assert tctx.master.has_log("Warning: Running proxyserver without nextlayer addon!", level="warn") await ps.shutdown_server() @@ -184,3 +184,15 @@ def test_options(): with pytest.raises(exceptions.OptionsError): tctx.configure(ps, stream_large_bodies="invalid") tctx.configure(ps, stream_large_bodies="1m") + + +async def test_startup_err(monkeypatch) -> None: + async def _raise(*_): + raise OSError("cannot bind") + + monkeypatch.setattr(asyncio, "start_server", _raise) + + ps = Proxyserver() + with taddons.context(ps) as tctx: + await ps.running() + await tctx.master.await_log("cannot bind", level="error") diff --git a/test/mitmproxy/tools/console/test_integration.py b/test/mitmproxy/tools/console/test_integration.py index cdc74681c..3d485484f 100644 --- a/test/mitmproxy/tools/console/test_integration.py +++ b/test/mitmproxy/tools/console/test_integration.py @@ -39,6 +39,7 @@ def console(monkeypatch) -> ConsoleTestMaster: async def make_master(): opts = mitmproxy.options.Options() m = ConsoleTestMaster(opts) + opts.server = False await m.running() return m return asyncio.run(make_master()) diff --git a/test/mitmproxy/tools/test_dump.py b/test/mitmproxy/tools/test_dump.py index c1e278c5c..5b9cb9aa9 100644 --- a/test/mitmproxy/tools/test_dump.py +++ b/test/mitmproxy/tools/test_dump.py @@ -2,7 +2,6 @@ from unittest import mock import pytest -from mitmproxy import log from mitmproxy import options from mitmproxy.tools import dump @@ -13,12 +12,6 @@ class TestDumpMaster: m = dump.DumpMaster(o, with_termlog=False, with_dumper=False) return m - async def test_has_error(self): - m = self.mkmaster() - ent = log.LogEntry("foo", "error") - m.addons.trigger(log.AddLogHook(ent)) - assert m.errorcheck.has_errored - @pytest.mark.parametrize("termlog", [False, True]) async def test_addons_termlog(self, termlog): with mock.patch('sys.stdout'):