Skip to content

Commit dc56665

Browse files
Ensure fallback returns something even if it has an error.
1 parent bf221eb commit dc56665

2 files changed

Lines changed: 79 additions & 28 deletions

File tree

src/labthings_fastapi/server/fallback.py

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import json
1212
from dataclasses import dataclass
1313
from pathlib import Path
14+
import logging
1415
from traceback import format_exception
1516
from typing import Any, TYPE_CHECKING
1617

@@ -24,6 +25,8 @@
2425
if TYPE_CHECKING:
2526
from . import ThingServer
2627

28+
LOGGER = logging.getLogger(__name__)
29+
2730
_TEMPLATE_PATH = Path(__file__).with_name("fallback.html.jinja")
2831

2932

@@ -46,6 +49,19 @@ class FallbackContext:
4649
"""Any logging history to show."""
4750

4851

52+
LAST_RESORT_PAGE = """
53+
<html>
54+
<head lang="en">
55+
<title>LabThings Internal Error</title>
56+
</head>
57+
<body>
58+
<h1>LabThings Internal Error</h1>
59+
<p>Couldn't start LabThings Server.</p>
60+
<p>A further error occurred when gathering context.</p>
61+
</body>
62+
"""
63+
64+
4965
class FallbackApp(FastAPI):
5066
"""A basic FastAPI application to serve a LabThings error page."""
5167

@@ -59,13 +75,17 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
5975
:param \**kwargs: is passed to `fastapi.FastAPI.__init__`\ .
6076
"""
6177
super().__init__(*args, **kwargs)
62-
# Handle dictionary config here for legacy reasons.
63-
self._context: FallbackContext | None = None
64-
self._env = Environment(
65-
loader=BaseLoader(),
66-
autoescape=select_autoescape(["html", "xml"]),
67-
)
68-
self.set_template_str(_TEMPLATE_PATH.read_text(encoding="utf-8"))
78+
try:
79+
# Handle dictionary config here for legacy reasons.
80+
self._context: FallbackContext | None = None
81+
self._env = Environment(
82+
loader=BaseLoader(),
83+
autoescape=select_autoescape(["html", "xml"]),
84+
)
85+
self.set_template_str(_TEMPLATE_PATH.read_text(encoding="utf-8"))
86+
except BaseException as e:
87+
# Catch any error and continue or there is no fallback server
88+
LOGGER.exception(e)
6989
self.html_code = 500
7090

7191
def set_context(self, context: FallbackContext) -> None:
@@ -101,26 +121,30 @@ def fallback_page(self) -> HTMLResponse:
101121
:return: The HTMLResponse for the fallback page.
102122
:raises RuntimeError: if the fallback context was never set.
103123
"""
104-
if self._context is None:
105-
raise RuntimeError("Not context set for fallback server.")
106-
107-
error_message, error_w_trace = _format_error_and_traceback(self._context)
108-
things = list(self._context.server.things) if self._context.server else []
109-
110-
if isinstance(self._context.config, ThingServerConfig):
111-
conf_str = self._context.config.model_dump_json(indent=2)
112-
else:
113-
conf_str = json.dumps(self._context.config, indent=2)
114-
115-
content = app._template.render(
116-
error_message=error_message,
117-
things=things,
118-
config=conf_str,
119-
traceback=error_w_trace,
120-
logginginfo=self._context.log_history,
121-
)
122-
123-
return HTMLResponse(content=content, status_code=app.html_code)
124+
try:
125+
if self._context is None:
126+
raise RuntimeError("Not context set for fallback server.")
127+
error_message, error_w_trace = _format_error_and_traceback(self._context)
128+
things = list(self._context.server.things) if self._context.server else []
129+
130+
if isinstance(self._context.config, ThingServerConfig):
131+
conf_str = self._context.config.model_dump_json(indent=2)
132+
else:
133+
conf_str = json.dumps(self._context.config, indent=2)
134+
135+
content = app._template.render(
136+
error_message=error_message,
137+
things=things,
138+
config=conf_str,
139+
traceback=error_w_trace,
140+
logginginfo=self._context.log_history,
141+
)
142+
143+
return HTMLResponse(content=content, status_code=app.html_code)
144+
except BaseException as e:
145+
# Catch any error and continue or there is no fallback server
146+
LOGGER.exception(e)
147+
return HTMLResponse(content=LAST_RESORT_PAGE, status_code=500)
124148

125149

126150
app = FallbackApp()

tests/test_fallback.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
verifies that it works as expected.
66
"""
77

8+
import logging
89
import re
910
from html import unescape
1011

@@ -13,7 +14,7 @@
1314

1415
from fastapi.testclient import TestClient
1516
import labthings_fastapi as lt
16-
from labthings_fastapi.server.fallback import app, FallbackContext
17+
from labthings_fastapi.server.fallback import app, FallbackApp, FallbackContext
1718
from labthings_fastapi.example_things import ThingThatCantStart
1819

1920
CONFIG_DICT = {
@@ -33,6 +34,32 @@ def reset_app_state():
3334
app._context = None
3435

3536

37+
def test_fallback_carries_on_even_with_init_error(mocker, caplog):
38+
"""Ensure fallback is created even if there is and error in __init__.
39+
40+
The exception should be raised.
41+
"""
42+
# Force set_template_str to fail
43+
mocker.patch.object(
44+
FallbackApp,
45+
"set_template_str",
46+
side_effect=RuntimeError("Mocked failure"),
47+
)
48+
with caplog.at_level(logging.ERROR):
49+
FallbackApp()
50+
assert len(caplog.records) == 1
51+
assert "Mocked failure" in caplog.records[0].message
52+
assert caplog.records[0].exc_info is not None
53+
54+
55+
def test_fallback_without_context():
56+
"""Test fallback server errors if context is not set at all."""
57+
with TestClient(app) as client:
58+
response = client.get("/")
59+
html = response.text
60+
assert "LabThings Internal Error" in html
61+
62+
3663
def test_fallback_redirect():
3764
"""Test that the redirect works."""
3865
# Need to set a context even if everything in it is empty.

0 commit comments

Comments
 (0)