Merge pull request #260 from rstudio/fix-error-thread

Make sure no exceptions escape our middleware stack

Author: jcheng5

Makefile

@@ -54,6 +54,9 @@ typings/uvicorn/__init__.pyi:
 check: typings/uvicorn/__init__.pyi ## type check with pyright
 	pyright
 
+check-old: typings/uvicorn/__init__.pyi ## type check with pyright
+	pyright --pythonversion=3.7
+
 lint: ## check style with flake8
 	flake8 --show-source --max-line-length=127 shiny tests examples
 

shiny/_app.py

@@ -7,6 +7,7 @@
 from htmltools import Tag, TagList, HTMLDocument, HTMLDependency, RenderedHTML
 
 import starlette.applications
+import starlette.exceptions
 import starlette.middleware
 import starlette.routing
 import starlette.websockets
@@ -16,6 +17,8 @@
 
 from ._autoreload import autoreload_url, InjectAutoreloadMiddleware
 from ._connection import Connection, StarletteConnection
+from ._shinyenv import is_pyodide
+from ._error import ErrorMiddleware
 from .html_dependencies import require_deps, jquery_deps, shiny_deps
 from .http_staticfiles import StaticFiles
 from .reactive import on_flushed
@@ -161,6 +164,13 @@ def init_starlette_app(self):
             middleware.append(
                 starlette.middleware.Middleware(InjectAutoreloadMiddleware)
             )
+        # In Pyodide mode, an HTTPException(404) being thrown resulted in
+        # some default error handler (that happened not to be async) being
+        # run in a threadpool, which Pyodide could not handle. So in Pyodide
+        # mode, install our own async error handler at the outermost layer
+        # that we can.
+        if is_pyodide:
+            middleware.append(starlette.middleware.Middleware(ErrorMiddleware))
 
         starlette_app = starlette.applications.Starlette(
             routes=routes,

shiny/_autoreload.py

@@ -132,7 +132,7 @@ async def rewrite_send(event: ASGISendEvent) -> None:
                         event["body"] = body.replace(b"</head>", self.script, 1)
                         body = b""  # Allow gc
                         intercept = False
-                    elif event["more_body"]:
+                    elif "more_body" in event and event["more_body"]:
                         # DO NOT send the response; wait for more data
                         return
                     else:

shiny/_error.py

@@ -0,0 +1,39 @@
+from typing import Dict, cast
+
+import starlette.exceptions as exceptions
+import starlette.responses as responses
+from starlette.types import ASGIApp, Receive, Scope, Send
+
+
+class ErrorMiddleware:
+    """Inserts shiny-autoreload.js into the head.
+
+    It's necessary to do it using middleware instead of in a nice htmldependency,
+    because we want autoreload to be effective even when displaying an error page.
+    """
+
+    def __init__(self, app: ASGIApp):
+        self.app = app
+
+    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
+        try:
+            return await self.app(scope, receive, send)
+        except exceptions.HTTPException as e:
+            resp = responses.PlainTextResponse(
+                e.detail,
+                e.status_code,
+                headers=cast(
+                    Dict[str, str],
+                    e.headers,  # pyright: ignore[reportUnknownMemberType]
+                ),
+                media_type="text/plain",
+            )
+            await resp(scope, receive, send)
+        except Exception as e:
+            # Seems super weird this is just going to stdout, should we use logger or
+            # at least stderr?
+            print("Unhandled error: " + str(e))
+            resp = responses.PlainTextResponse(
+                "An internal server error occurred", 500, media_type="text/plain"
+            )
+            await resp(scope, receive, send)