Skip to content

Commit

Permalink
HH-237884 don't swallow exceptions on page import
Browse files Browse the repository at this point in the history
  • Loading branch information
712u3 committed Nov 13, 2024
1 parent bdb2d70 commit a8a8666
Show file tree
Hide file tree
Showing 36 changed files with 174 additions and 186 deletions.
14 changes: 8 additions & 6 deletions frontik/app.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import asyncio
import importlib
import inspect
import logging
import multiprocessing
import os
Expand Down Expand Up @@ -35,9 +35,11 @@
)
from frontik.service_discovery import MasterServiceDiscovery, ServiceDiscovery, WorkerServiceDiscovery
from frontik.tornado_connection_handler import LegacyTornadoConnectionHandler, TornadoConnectionHandler
from frontik.util import Sentinel
from frontik.version import version as frontik_version

app_logger = logging.getLogger('app_logger')
_DEFAULT_ARG = Sentinel()


class FrontikApplication(httputil.HTTPServerConnectionDelegate):
Expand All @@ -46,13 +48,12 @@ class FrontikApplication(httputil.HTTPServerConnectionDelegate):
class DefaultConfig:
pass

def __init__(self, app_module_name: Optional[str] = None) -> None:
def __init__(self, app_module_name: Union[None, str, Sentinel] = _DEFAULT_ARG) -> None:
self.start_time = time.time()
self.patch_anyio()

self.app_module_name: str = app_module_name or self.__class__.__module__
app_module = importlib.import_module(self.app_module_name)
self.app_root = os.path.dirname(str(app_module.__file__))
self.app_module_name: str = app_module_name if isinstance(app_module_name, str) else self.__class__.__module__
self.app_root = os.path.dirname(inspect.getfile(self.__class__))
if options.service_name is None:
options.service_name = self.app_module_name.rsplit('.', 1)[-1]
self.app_name = options.service_name
Expand All @@ -69,7 +70,8 @@ def __init__(self, app_module_name: Optional[str] = None) -> None:
count_down_lock = multiprocessing.Lock()
self.worker_state = WorkerState(init_workers_count_down, master_done, count_down_lock) # type: ignore

import_all_pages(self.app_module_name)
if app_module_name is not None:
import_all_pages(self.app_module_name)

self.settings: dict = {}

Expand Down
3 changes: 2 additions & 1 deletion frontik/config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ def parse_command_line(options: Options, allowed_options: Iterable) -> None:
if option.type == bool:
setattr(options, name, True)
else:
raise Exception('Option %r requires a value' % name)
msg = f'Option {name!r} requires a value'
raise Exception(msg)

if option.type == bool or get_args(option.type) == (bool, type(None)):
setattr(options, name, value.lower() not in ('false', '0', 'f'))
Expand Down
2 changes: 1 addition & 1 deletion frontik/debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ def transform_chunk(

debug_log_data = request_context.get_debug_log_handler().produce_all() # type: ignore
debug_log_data.set('code', str(int(response.status_code)))
debug_log_data.set('handler-name', handler_name if handler_name else 'unknown handler')
debug_log_data.set('handler-name', handler_name or 'unknown handler')
debug_log_data.set('started', _format_number(tornado_request._start_time))
debug_log_data.set('request-id', str(tornado_request.request_id))
debug_log_data.set('stages-total', _format_number((time.time() - tornado_request._start_time) * 1000))
Expand Down
8 changes: 4 additions & 4 deletions frontik/loggers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ def format_stack_trace(self, record: logging.LogRecord) -> str:
record.exc_text = self.formatException(record.exc_info)
if record.exc_text:
if stack_trace[-1:] != '\n':
stack_trace = stack_trace + '\n'
stack_trace = stack_trace + record.exc_text
stack_trace += '\n'
stack_trace += record.exc_text
if record.stack_info:
if stack_trace[-1:] != '\n':
stack_trace = stack_trace + '\n'
stack_trace = stack_trace + self.formatStack(record.stack_info)
stack_trace += '\n'
stack_trace += self.formatStack(record.stack_info)

return stack_trace

Expand Down
19 changes: 10 additions & 9 deletions frontik/routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,21 @@ def _iter_submodules(path: MutableSequence[str], prefix: str = '') -> Generator:
def import_all_pages(app_module: str) -> None:
"""Import all pages on startup"""

try:
pages_package = importlib.import_module(f'{app_module}.pages')
except ModuleNotFoundError:
_spec = importlib.util.find_spec(f'{app_module}.pages')
if _spec is None:
routing_logger.warning('There is no pages module')
return

pages_package = importlib.import_module(f'{app_module}.pages')

for _, name, __ in _iter_submodules(pages_package.__path__):
full_name = pages_package.__name__ + '.' + name
try:
importlib.import_module(full_name)
except ModuleNotFoundError:

_spec = importlib.util.find_spec(full_name)
if _spec is None:
continue
except Exception as ex:
raise RuntimeError('failed on import page %s %s', full_name, ex)

importlib.import_module(full_name)


router = FastAPIRouter()
Expand All @@ -101,7 +102,7 @@ def _find_fastapi_route(scope: dict) -> Optional[APIRoute]:
if route_path.endswith('/'):
scope['path'] = scope['path'].rstrip('/')
else:
scope['path'] = scope['path'] + '/'
scope['path'] += '/'

for route in _fastapi_routes:
match, child_scope = route.matches(scope)
Expand Down
2 changes: 1 addition & 1 deletion frontik/service_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def _make_service_id(options: Options, *, service_name: Optional[str], hostname:
def _create_http_check(options: Options, address: Optional[str]) -> dict:
check_host = options.consul_check_host
if not check_host:
check_host = address if address else '127.0.0.1'
check_host = address or '127.0.0.1'
http_check = Check.http(
f'http://{check_host}:{options.port}/status',
f'{options.consul_http_check_interval_sec}s',
Expand Down
2 changes: 1 addition & 1 deletion frontik/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def setup_mock_http_client(self, frontik_app, passthrow_hosts, with_tornado_mock
self.mock_http_client = mock_http_client
yield self.mock_http_client

@pytest.fixture()
@pytest.fixture
def passthrow_hosts(self):
return ['http://127.0.0.1', 'http://localhost']

Expand Down
4 changes: 2 additions & 2 deletions frontik/timeout_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import logging
import time
from collections import namedtuple
from collections import UserDict, namedtuple
from functools import partial
from typing import TYPE_CHECKING, Optional

Expand All @@ -22,7 +22,7 @@
)


class TimeoutCounter(dict):
class TimeoutCounter(UserDict):
def increment(self, k: LoggingData, already_spent_ms: float) -> None:
count, max_already_spent_ms = super().__getitem__(k)
super().__setitem__(k, (count + 1, max(already_spent_ms, max_already_spent_ms)))
Expand Down
4 changes: 4 additions & 0 deletions frontik/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
logger = logging.getLogger('util')


class Sentinel:
pass


def safe_template(format_string: str, **kwargs: Any) -> str:
"""Safe templating using PEP-292 template strings
(see https://docs.python.org/3/library/string.html#template-strings).
Expand Down
Loading

0 comments on commit a8a8666

Please sign in to comment.