From 14dcdb7e501d5a9fc6fb548bb15cf92f12c4791d Mon Sep 17 00:00:00 2001 From: Aran-Fey Date: Tue, 4 Jun 2024 19:23:38 +0200 Subject: [PATCH] fix inconsistent asset urls --- frontend/code/components/devToolsConnector.ts | 2 +- frontend/code/components/plot.ts | 2 +- frontend/index.html | 7 ++++++- rio/app_server/abstract_app_server.py | 12 ++++++----- rio/app_server/fastapi_server.py | 13 ++++-------- rio/byte_serving.py | 4 ++++ rio/session.py | 21 +++++++++++++++---- 7 files changed, 40 insertions(+), 21 deletions(-) diff --git a/frontend/code/components/devToolsConnector.ts b/frontend/code/components/devToolsConnector.ts index dbd003e4..29cafb75 100644 --- a/frontend/code/components/devToolsConnector.ts +++ b/frontend/code/components/devToolsConnector.ts @@ -22,7 +22,7 @@ export class DevToolsConnectorComponent extends ComponentBase { element.target = '_blank'; element.classList.add('rio-dev-tools-connector'); element.innerHTML = ` - +
Rio
`; diff --git a/frontend/code/components/plot.ts b/frontend/code/components/plot.ts index 7c0e38f2..cb815b2c 100644 --- a/frontend/code/components/plot.ts +++ b/frontend/code/components/plot.ts @@ -39,7 +39,7 @@ function withPlotly(callback: () => void): void { // Otherwise fetch plotly and call the callback when it's done console.debug('Fetching plotly.js'); let script = document.createElement('script'); - script.src = '/rio/asset/special/plotly.min.js'; + script.src = '/rio/assets/special/plotly.min.js'; fetchPlotlyPromise = new Promise((resolve) => { script.onload = () => { diff --git a/frontend/index.html b/frontend/index.html index a6109647..7f4b930e 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -18,7 +18,12 @@ white screen globalThis.initialMessages = '{initial_messages}'; - // Create a Promise that resolves when the CSS is done loading + // Create a Promise that resolves when the CSS is done loading. + // + // Note: This may look like a race condition, since the assignments + // happen inside a Promise. but according to MDN this is exactly + // equivalent to `Promise.withResolvers`. (Which we're not using + // because it's too new.) let resolveCssLoaded, rejectCssLoaded; const cssLoaded = new Promise((resolve, reject) => { resolveCssLoaded = resolve; diff --git a/rio/app_server/abstract_app_server.py b/rio/app_server/abstract_app_server.py index eaf6761c..6fc90b85 100644 --- a/rio/app_server/abstract_app_server.py +++ b/rio/app_server/abstract_app_server.py @@ -240,6 +240,13 @@ class AbstractAppServer(abc.ABC): if not isinstance(attachment, user_settings_module.UserSettings): sess.attach(attachment) + # Start listening for incoming messages. This should happen before + # `on_session_start` is called, so that we don't deadlock in case + # someone calls a method that requires a response from the client. + self._session_serve_tasks[sess] = asyncio.create_task( + self._serve_session(sess) + ) + # Trigger the `on_session_start` event. # # Since this event is often used for important initialization tasks like @@ -324,11 +331,6 @@ class AbstractAppServer(abc.ABC): # Send the first `updateComponentStates` message await sess._refresh() - # Start listening for incoming messages - self._session_serve_tasks[sess] = asyncio.create_task( - self._serve_session(sess) - ) - return sess async def _serve_session(self, sess: rio.Session) -> None: diff --git a/rio/app_server/fastapi_server.py b/rio/app_server/fastapi_server.py index 6b250549..990a5a4f 100644 --- a/rio/app_server/fastapi_server.py +++ b/rio/app_server/fastapi_server.py @@ -8,7 +8,6 @@ import inspect import io import json import logging -import mimetypes import secrets import traceback import weakref @@ -201,22 +200,22 @@ class FastapiServer(fastapi.FastAPI, AbstractAppServer): self._serve_frontend_asset, ) self.add_api_route( - "/rio/asset/special/{asset_id:path}", + "/rio/assets/special/{asset_id:path}", self._serve_special_asset, methods=["GET"], ) self.add_api_route( - "/rio/asset/hosted/{asset_id:path}", + "/rio/assets/hosted/{asset_id:path}", self._serve_hosted_asset, methods=["GET"], ) self.add_api_route( - "/rio/asset/user/{asset_id:path}", + "/rio/assets/user/{asset_id:path}", self._serve_user_asset, methods=["GET"], ) self.add_api_route( - "/rio/asset/temp/{asset_id:path}", + "/rio/assets/temp/{asset_id:path}", self._serve_temp_asset, methods=["GET"], ) @@ -544,10 +543,6 @@ Sitemap: {request_url.with_path("/rio/sitemap")} ) response.headers["content-encoding"] = "gzip" - media_type = mimetypes.guess_type(asset_id, strict=False)[0] - if media_type: - response.headers["content-type"] = media_type - return response @add_cache_headers diff --git a/rio/byte_serving.py b/rio/byte_serving.py index be8c796d..ebf12268 100644 --- a/rio/byte_serving.py +++ b/rio/byte_serving.py @@ -5,6 +5,7 @@ Based on a comment here: https://github.com/tiangolo/fastapi/issues/1240#issuecomment-1055396884 """ +import mimetypes from pathlib import Path from typing import * # type: ignore @@ -89,6 +90,9 @@ def range_requests_response( ), } + if media_type is None: + media_type = mimetypes.guess_type(file_path, strict=False)[0] + if media_type is not None: headers["content-type"] = media_type diff --git a/rio/session.py b/rio/session.py index 71facbda..d735985a 100644 --- a/rio/session.py +++ b/rio/session.py @@ -198,6 +198,13 @@ class Session(unicall.Unicall): # the tasks are cancelled when the session is closed. self._running_tasks: set[asyncio.Task[object]] = set() + # Keeps track of tasks that must be awaited before an + # `updateComponentStates` message is sent to the client. (An example of + # such a task is registering a new font.) + self._tasks_that_need_to_finish_before_update_component_states: set[ + asyncio.Task[object] + ] = set() + # Keeps track of all PageView instances in this session. PageViews add # themselves to this. self._page_views: weakref.WeakSet[rio.PageView] = weakref.WeakSet() @@ -1226,6 +1233,13 @@ window.history.{method}(null, "", {json.dumps(str(active_page_url))}) else: root_component_id = None + # Make sure all + # `_tasks_that_need_to_finish_before_update_component_states` are + # finished + tasks = self._tasks_that_need_to_finish_before_update_component_states + self._tasks_that_need_to_finish_before_update_component_states = set() + await asyncio.gather(*tasks) + # Send the new state to the client await self._remote_update_component_states( delta_states, root_component_id @@ -1721,10 +1735,9 @@ window.history.{method}(null, "", {json.dumps(str(active_page_url))}) font_assets.append(asset) - # FIXME: Offloading this into a task creates a race condition... the - # `updateComponentStates` message could be sent before the - # `registerFont` message, then everything would use the wrong fonts - self.create_task(self._remote_register_font(font_name, urls)) # type: ignore + self._tasks_that_need_to_finish_before_update_component_states.add( + self.create_task(self._remote_register_font(font_name, urls)) + ) self._registered_font_names[font] = font_name self._registered_font_assets[font] = font_assets