From da9cc2ffb3ba5b6cefefd861df82778e7792fb01 Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Tue, 28 Jul 2026 11:51:01 -0500 Subject: [PATCH 1/9] feat(chat-with-content): scope to the viewer, surface errors, harden startup --- extensions/chat-with-content/CHANGELOG.md | 64 ++ extensions/chat-with-content/app.py | 598 ++++++++++++++---- extensions/chat-with-content/manifest.json | 4 +- extensions/chat-with-content/requirements.txt | 2 +- 4 files changed, 538 insertions(+), 130 deletions(-) diff --git a/extensions/chat-with-content/CHANGELOG.md b/extensions/chat-with-content/CHANGELOG.md index 3cc36f7a..765994f1 100644 --- a/extensions/chat-with-content/CHANGELOG.md +++ b/extensions/chat-with-content/CHANGELOG.md @@ -7,12 +7,76 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.0.8] - 2026-07-17 +### Added + +- An in-app note explaining that the app runs as the signed-in viewer, reads + content with their own permissions via the Visitor API Key, and stores no admin + key. (#447) +- Clear, persistent error notifications when content can't be listed or opened + from Connect, showing the reason instead of leaving the selector silently + empty, plus a message when you have no content available to chat with. (#447) + +### Changed + +- The setup screen now shows only the step still missing rather than repeating + both. (#447) +- Refreshed the default model names to Claude Sonnet 4.5. (#447) +- Skip the AWS Bedrock credential probe at startup when a chat provider is + configured, and cap it with a timeout when it does run, so a slow or + unreachable Bedrock endpoint can't delay or hang the app's startup. (#447) +- Show the actual error in the chat when a request fails, instead of a generic + message. (#447) + ### Fixed - Guarded against content with a missing, malformed, or timezone-naive deployment time, which could previously leave the whole content list empty. (#446) - Close a code block that truncation cut open, so the model reads the truncation note as a note rather than as more code. (#446) +- Handle a missing or misconfigured chat provider instead of crashing: an + unconfigured provider shows the setup screen, and a configured one that can't + start (a bad model name or missing API key) shows a readable error. (#447) +- Show a readable error if the selected content can't be read to summarize it, + instead of silently doing nothing. (#447) +- Show a readable error when Connect gives no URL for the selected content, rather + than failing with nothing shown. (#447) +- Show a clear error when your Connect session can't be read, instead of silently + running with the wrong identity and failing later, and never fall back to listing + the deployer's content as if it were yours. (#447) +- Name the real reason a session can't be read rather than showing setup steps that + wouldn't fix it: being signed out, and the server having OAuth integrations + disabled, are each reported as themselves. (#447) +- Don't summarize an unrelated page if the content frame redirects cross-origin + (for example to an external login). (#447) +- Re-summarize when you switch to a different content item whose rendered HTML is + byte-identical to the previous one, instead of leaving the earlier summary up. (#447) +- Clear the previous item's summary when you switch content, so the chat no longer + shows a summary the model has already forgotten. (#447) +- Stream one reply at a time, so switching content or asking a question while a + reply is still streaming no longer interleaves two replies, drops part of the + new one, or sends the model a half-written conversation. (#447) +- Forget an exchange whose request failed, so a single failed reply no longer + makes every later question fail until you switch content or reload. (#447) +- Cap the page content the browser sends, so selecting a very large report + summarizes it instead of disconnecting the app. (#447) +- Stop generating a reply once you close or reload the page, instead of letting it + run to completion unseen. (#447) +- Stop waiting on a reply that stalls, so a provider that goes quiet no longer + leaves the app looking frozen with the chat input disabled. (#447) +- Convert the selected page outside the reactive work the server serializes, so + opening a large report no longer pauses every other session on that worker. (#447) +- Finish answering a question you asked even if you switch content while it is + streaming, instead of replacing the answer with a blank reply. (#447) +- Empty the chat when the selected content can't be read, so later answers can't be + drawn from the item that is no longer on screen. (#447) +- Let a reload retry a summary whose request failed, instead of mistaking it for a + repeat of the same page. (#447) +- Let the worker exit promptly when a credential probe is still hanging, so + restarting the content isn't held up by it. (#447) +- Summarize a selection once even when its page loads more than once, instead of + spending a second request to say the same thing. (#447) +- Truncated large content before sending it to the model so a big page can't + overflow the context window. (#447) ## [0.0.7] - 2026-06-15 diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index 3ad7abc2..e04fe266 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -1,48 +1,74 @@ +import asyncio import os +import threading from posit import connect -from posit.connect.content import ContentItem -from posit.connect.errors import ClientError -from chatlas import ChatAuto, ChatBedrockAnthropic, SystemTurn, UserTurn +from chatlas import ChatAuto, ChatBedrockAnthropic import markdownify from shiny import App, Inputs, Outputs, Session, ui, reactive, render -from helpers import time_since_deployment +from helpers import ( + content_choice_label, + content_ready, + is_chattable_content, + resolve_visitor_client, + running_on_connect, + truncate_for_context, +) + +# Zero-config fallback model, used only when no LLM provider is configured. Bedrock +# picks up credentials from an instance role, so it needs no API key. +BEDROCK_MODEL = "us.anthropic.claude-sonnet-4-5-20250929-v1:0" + +# Cap the startup Bedrock probe so a reachable-but-slow endpoint can't hang the worker. +BEDROCK_PROBE_TIMEOUT_SECONDS = 10 + +# Give up on a reply that stalls between chunks. Streams run one at a time, so a +# provider that stops sending would otherwise hold that slot and leave the app +# looking frozen with its input disabled. Generous, because the first chunk for a +# large page can legitimately take a while, but far below the provider SDKs' own +# ten-minute default. +STREAM_STALL_TIMEOUT_SECONDS = 120 def check_aws_bedrock_credentials(): - # Check if AWS credentials are available in the environment - # that can be used to access Bedrock - try: - chat = ChatBedrockAnthropic( - model="us.anthropic.claude-sonnet-4-20250514-v1:0", - ) - chat.chat("test", echo="none") + # Probe for usable Bedrock credentials by making a real (throwaway) Bedrock call. + # Bedrock is the zero-config fallback: this only runs when no provider is set via + # CHATLAS_CHAT_PROVIDER_MODEL, so an explicit choice is never probed over. + # The probe makes a live network call at import, so run it under a timeout: a + # reachable-but-slow Bedrock (partial credentials, throttling) must not block + # worker startup. On timeout, fall back to the setup screen. + outcome = {} + + def _probe(): + try: + ChatBedrockAnthropic(model=BEDROCK_MODEL).chat("test", echo="none") + outcome["ok"] = True + except Exception as e: # noqa: BLE001 - reported below, never raised + outcome["error"] = e + + # A daemon thread, so a probe still blocked on a socket can't hold up process + # exit when Connect restarts this content. A pooled thread is joined at exit. + thread = threading.Thread(target=_probe, daemon=True) + thread.start() + thread.join(BEDROCK_PROBE_TIMEOUT_SECONDS) + if outcome.get("ok"): return True - except Exception as e: - print( - f"AWS Bedrock credentials check failed and will fall back to checking for values for the CHATLAS_CHAT_PROVIDER_MODEL env var. Err: {e}" - ) - return False + reason = outcome.get("error", "timed out") + print( + f"AWS Bedrock credential probe failed or timed out; with no LLM provider " + f"configured, the app will show the setup screen. Err: {reason}" + ) + return False def fetch_connect_content_list(client: connect.Client): - content_list: list[ContentItem] = client.content.find(include=["owner", "tags"]) - app_modes = ["jupyter-static", "quarto-static", "rmd-static", "static"] - filtered_content_list = [] - for content in content_list: - if ( - content.app_mode in app_modes - and content.app_role != "none" - and content.content_category != "pin" - ): - filtered_content_list.append(content) - - return filtered_content_list - - -setup_ui = ui.page_fillable( - ui.tags.style( - """ + content_list = client.content.find(include=["owner", "tags"]) + return [item for item in content_list if is_chattable_content(item)] + + +# Shared styling for the setup screen. +_SETUP_STYLE = ui.tags.style( + """ body { padding: 0; margin: 0; @@ -118,44 +144,95 @@ def fetch_connect_content_list(client: connect.Client): } } """ +) + + +# The two setup steps, each shown only while its piece is still unconfigured. +_LLM_SETUP_SECTION = ( + ui.h2("LLM API", class_="setup-section-title"), + ui.div( + ui.HTML( + "This app needs the CHATLAS_CHAT_PROVIDER_MODEL environment variable " + "and a matching LLM API key. In the content settings, on the " + "Advanced tab, add both of them under Environment Variables. " + "On AWS Bedrock with an instance role, credentials are detected automatically and no " + "variables are needed. For more information, " + 'see the chatlas documentation.' + ), + class_="setup-description", + ), + ui.h3("Example Environment Variables for OpenAI API", class_="setup-section-title"), + ui.pre( + """Name: CHATLAS_CHAT_PROVIDER_MODEL +Value: openai/gpt-4o + +Name: OPENAI_API_KEY +Value: """, + class_="setup-code-block", ), +) + +_INTEGRATION_SETUP_SECTION = ( + ui.h2("Connect Visitor API Key", class_="setup-section-title"), ui.div( + ui.HTML( + 'This app needs a "Connect Visitor API Key" integration so it can list and read ' + "content as the signed-in viewer. In the content settings, on the " + 'Access tab, add the "Connect Visitor API Key" integration under ' + "Integrations. For more information, " + 'see the OAuth Integrations documentation.' + ), + class_="setup-description", + ), +) + + +def setup_ui(need_llm: bool, need_integration: bool): + # Show only the piece(s) still unconfigured, so a partially configured app doesn't + # repeat setup steps the publisher has already done. + sections = [] + if need_llm: + sections.extend(_LLM_SETUP_SECTION) + if need_integration: + sections.extend(_INTEGRATION_SETUP_SECTION) + return ui.page_fillable( + _SETUP_STYLE, ui.div( - ui.h1("Setup", class_="setup-title"), - ui.h2("LLM API", class_="setup-section-title"), ui.div( - ui.HTML( - "This app requires the CHATLAS_CHAT_PROVIDER_MODEL environment variable to be " - "set along with an LLM API Key in the content settings. Please set them in your environment before running the app. " - 'See the documentation for more details on which arguments can be set for each Chatlas provider.' - ), - class_="setup-description", - ), - ui.h3("Example for OpenAI API", class_="setup-section-title"), - ui.pre( - """CHATLAS_CHAT_PROVIDER_MODEL = "openai/gpt-4o" -OPENAI_API_KEY = "" """, - class_="setup-code-block", + ui.h1("Setup", class_="setup-title"), + *sections, + class_="setup-card", ), + class_="setup-container", + ), + fillable_mobile=True, + fillable=True, + ) + + +def error_ui(detail: str, message: str | None = None): + # Shown when the app can't start for the viewer (e.g. the session couldn't be + # read, or the chat provider couldn't be initialized), so the failure states why + # in plain language instead of crashing. message carries the underlying error + # when there is one; some failures are entirely explained by the detail. + return ui.page_fillable( + _SETUP_STYLE, + ui.div( ui.div( - ui.HTML( - 'For other provider examples (Azure OpenAI, Anthropic, AWS Bedrock, etc.), see the ' - 'README.' + ui.h1("Something went wrong", class_="setup-title"), + ui.div( + detail, + class_="setup-description", ), - class_="setup-description", - ), - ui.h2("Connect Visitor API Key", class_="setup-section-title"), - ui.div( - "Before you are able to use this app, you need to add a Connect Visitor API Key integration in the content settings.", - class_="setup-description", + ui.pre(message, class_="setup-code-block") if message else None, + class_="setup-card", ), - class_="setup-card", + class_="setup-container", ), - class_="setup-container", - ), - fillable_mobile=True, - fillable=True, -) + fillable_mobile=True, + fillable=True, + ) + app_ui = ui.page_sidebar( # Sidebar with content selector and chat @@ -164,7 +241,11 @@ def fetch_connect_content_list(client: connect.Client): ui.p( "Use this app to select content and ask questions about it. It currently supports static/rendered content." ), - ui.input_selectize("content_selection", "", choices=[], width="100%"), + # Show the viewer how their identity and permissions drive the app + ui.output_ui("identity_note"), + # Rendered with its choices (see content_selector) rather than declared empty + # and filled with update_select, so the dropdown paints already populated. + ui.output_ui("content_selector"), ui.chat_ui( "chat", placeholder="Type your question here...", @@ -183,14 +264,32 @@ def fetch_connect_content_list(client: connect.Client): ), # Add JavaScript to handle iframe updates and content extraction ui.tags.script(""" + // Cap the scraped HTML before sending it. A page carrying megabytes of + // embedded scripts and data would exceed Shiny's websocket message limit, + // which drops the whole session rather than reporting an error, and would + // block the worker while it was converted to markdown. This is only a + // transport limit; the server truncates the markdown again to fit the + // model's context window. + var MAX_HTML_CHARS = 1000000; + window.Shiny.addCustomMessageHandler('update-iframe', function(message) { var iframe = document.getElementById('content_frame'); iframe.src = message.url; iframe.onload = function() { - var iframeDoc = iframe.contentWindow.document; - var content = iframeDoc.documentElement.outerHTML; - Shiny.setInputValue('iframe_content', content); + var content; + try { + content = iframe.contentWindow.document.documentElement.outerHTML; + } catch (e) { + // Cross-origin: the frame loaded from a different origin (e.g. an + // external redirect), so we can't read it to summarize. Tell the + // server so it can inform the viewer instead of failing silently. + Shiny.setInputValue('iframe_read_failed', Date.now(), {priority: 'event'}); + return; + } + // priority 'event' so selecting a different item whose HTML is + // byte-identical to the last still re-fires and re-summarizes. + Shiny.setInputValue('iframe_content', content.slice(0, MAX_HTML_CHARS), {priority: 'event'}); }; }); """), @@ -204,25 +303,36 @@ def fetch_connect_content_list(client: connect.Client): CHATLAS_CHAT_PROVIDER = os.getenv("CHATLAS_CHAT_PROVIDER") CHATLAS_CHAT_PROVIDER_MODEL = os.getenv("CHATLAS_CHAT_PROVIDER_MODEL") CHATLAS_CHAT_ARGS = os.getenv("CHATLAS_CHAT_ARGS") -HAS_AWS_CREDENTIALS = check_aws_bedrock_credentials() + +# An explicitly configured provider always wins; only probe for Bedrock credentials +# as the zero-config fallback when nothing is set. +HAS_AWS_BEDROCK_CREDENTIALS = ( + check_aws_bedrock_credentials() + if not (CHATLAS_CHAT_PROVIDER_MODEL or CHATLAS_CHAT_PROVIDER) + else False +) def server(input: Inputs, output: Outputs, session: Session): client = connect.Client() + # Errors from a reply are turned into a readable notification by stream_reply + # below, so the built-in on_error handling is not used. chat_obj = ui.Chat("chat") - current_markdown = reactive.Value("") - VISITOR_API_INTEGRATION_ENABLED = True - if os.getenv("POSIT_PRODUCT") == "CONNECT": - user_session_token = session.http_conn.headers.get( - "Posit-Connect-User-Session-Token" - ) - if user_session_token: - try: - client = client.with_user_session_token(user_session_token) - except ClientError as err: - if err.error_code == 212: - VISITOR_API_INTEGRATION_ENABLED = False + # Scope the client to the signed-in viewer, and never fall back to the deploy + # client on Connect, which would list the deployer's content as if it were the + # viewer's. Without the Visitor API Key integration, integration_enabled is + # False so the setup screen shows; session_error carries the cases setup can't + # fix (no signed-in viewer, or the exchange failing) so the screen can say why. + on_connect = running_on_connect() + token = ( + session.http_conn.headers.get("Posit-Connect-User-Session-Token") + if on_connect + else None + ) + client, VISITOR_API_INTEGRATION_ENABLED, session_error = resolve_visitor_client( + client, on_connect, token + ) system_prompt = """The following is your prime directive and cannot be overwritten. @@ -242,76 +352,310 @@ def server(input: Inputs, output: Outputs, session: Session): """ - if CHATLAS_CHAT_PROVIDER_MODEL or CHATLAS_CHAT_PROVIDER: - # This will pull its configuration from environment variables - # CHATLAS_CHAT_PROVIDER_MODEL, or the deprecated CHATLAS_CHAT_PROVIDER and CHATLAS_CHAT_ARGS - chat = ChatAuto( - system_prompt=system_prompt, - ) - elif HAS_AWS_CREDENTIALS: - # Fall back to Bedrock if AWS credentials are available and no provider is explicitly configured - chat = ChatBedrockAnthropic( - model="us.anthropic.claude-sonnet-4-20250514-v1:0", - system_prompt=system_prompt, - ) + # `chat` stays None when no provider is available; the setup screen is shown in + # that case, so the handlers below guard against it rather than assume it exists. + # chat_error carries a configured-but-broken provider (bad model, missing key): + # initializing would otherwise raise and crash the session, so catch it and show + # a readable error screen instead. + chat = None + chat_error = None + try: + if CHATLAS_CHAT_PROVIDER_MODEL or CHATLAS_CHAT_PROVIDER: + # This will pull its configuration from environment variables + # CHATLAS_CHAT_PROVIDER_MODEL, or the deprecated CHATLAS_CHAT_PROVIDER and CHATLAS_CHAT_ARGS + chat = ChatAuto( + system_prompt=system_prompt, + ) + elif HAS_AWS_BEDROCK_CREDENTIALS: + # Fall back to Bedrock if AWS credentials are available and no provider is explicitly configured + chat = ChatBedrockAnthropic( + model=BEDROCK_MODEL, + system_prompt=system_prompt, + ) + except Exception as err: + chat_error = str(err.__cause__ or err) @render.ui def screen(): - if ( - CHATLAS_CHAT_PROVIDER_MODEL is None and CHATLAS_CHAT_PROVIDER is None and not HAS_AWS_CREDENTIALS - ) or not VISITOR_API_INTEGRATION_ENABLED: - return setup_ui - else: - return app_ui - - # Set up content selector + # An unusable session blocks everything, so show it before anything else. + # The helper supplies the detail because the reason differs: no signed-in + # viewer reads differently from an exchange that failed. + if session_error is not None: + return error_ui(*session_error) + # A configured-but-broken chat provider can't be fixed from the setup screen, + # so say what actually failed rather than showing setup steps. + if chat_error is not None: + return error_ui( + "Couldn't start the chat provider, so the app can't answer " + "questions about your content. The error was:", + chat_error, + ) + # Show only the setup step(s) still missing; otherwise the app itself. + need_llm = chat is None + need_integration = not VISITOR_API_INTEGRATION_ENABLED + if need_llm or need_integration: + return setup_ui(need_llm, need_integration) + return app_ui + + # Explain in-app how identity and permissions flow, using the viewer's own name + @render.ui + def identity_note(): + name = "you" + try: + me = client.me + name = ( + f"{me.get('first_name', '')} {me.get('last_name', '')}".strip() + or me.get("username") + or "you" + ) + except Exception: + pass + return ui.p( + "Signed in as ", + ui.strong(name), + ", resolved from your Connect session. Content is listed and read " + "with your own permissions through a Connect Visitor API Key. No " + "admin key is stored, and answers draw only on the content you select.", + class_="text-muted small", + ) + + # The content selector's choices. Held in a reactive value and rendered directly + # by content_selector, so the dropdown is populated when it first paints instead + # of racing an update_select message against the dynamically rendered screen. + selector_choices = reactive.Value({}) + + # Load the viewer's content into the selector. @reactive.Effect def _(): - content_list = fetch_connect_content_list(client) - content_choices = { - item.guid: f"{item.title or item.name} - {item.owner.first_name} {item.owner.last_name} {time_since_deployment(item.last_deployed_time)}" - for item in content_list - } - ui.update_select( + # This effect runs regardless of which screen is rendered, so it gates on + # the same readiness the setup screen uses. Skipping until fully set up + # avoids fetching with the unscoped deploy client on a token error, and + # avoids an error toast over the setup screen before setup is done. + if not content_ready(session_error, chat, VISITOR_API_INTEGRATION_ENABLED): + return + try: + content_list = fetch_connect_content_list(client) + # Build the labels inside the try too, so a bad item surfaces the error + # rather than silently leaving the selector empty. + choices = { + item["guid"]: content_choice_label(item) for item in content_list + } + except Exception as err: + cause = err.__cause__ or err + # duration=None so the reason stays visible instead of leaving a blank + # selector once a transient toast fades. + ui.notification_show( + f"Couldn't load your content from Connect: {cause}", + type="error", + duration=None, + ) + return + if not choices: + ui.notification_show( + "You don't have any content available to chat with.", + type="message", + duration=None, + ) + return + selector_choices.set({"": "Select content", **choices}) + + @render.ui + def content_selector(): + return ui.input_selectize( "content_selection", - choices={"": "Select content", **content_choices}, + "", + choices=selector_choices.get(), + width="100%", ) + # Selecting new content supersedes a reply that is still streaming for the + # previous item. A plain holder (not reactive) so the streaming task below can + # read it without a reactive context. + content_token = {"n": 0} + + # What the last summary was built from, so a repeated frame load doesn't buy a + # second identical summary. + last_summary = {"token": None, "markdown": None} + + # Every reply streams through this one extended task. Shiny queues a call made + # while the task is running and starts it only once the previous one has fully + # finished, so exactly one stream is ever in flight. Two at once would fight over + # the chat object: the transcript holds one stream at a time and queues the + # other's chunks behind it (dropping some), and each request sends the provider + # the other's half-written turn. + async def reset_conversation(): + # Clear the transcript and the model's memory together, so the chat is never + # left answering from an item that is no longer in the frame. + await chat_obj.clear_messages() + chat.set_turns([]) + last_summary["token"], last_summary["markdown"] = None, None + + @reactive.extended_task + async def stream_reply(page: str | None, token: int, new_content: bool): + # For a summary, `page` is the scraped HTML; for a question it is the text the + # viewer typed. A summary with no page means the frame couldn't be read. + # + # Only a summary is dropped when a newer selection supersedes it. A question + # has already been shown in the transcript with a spinner, so dropping it + # would leave a blank answer there with nothing explaining why. + if new_content and token != content_token["n"]: + return + try: + if new_content: + if page is None: + await reset_conversation() + return + try: + # Converted here rather than in the effect that scraped it: an + # effect runs inside the process-wide reactive lock and holds it + # across every await, so converting a large page there would + # stall every session on this worker. An extended task runs + # outside that lock. + markdown = truncate_for_context( + await asyncio.to_thread( + markdownify.markdownify, page, heading_style="atx" + ) + ) + except Exception as err: + await reset_conversation() + cause = err.__cause__ or err + ui.notification_show( + f"Couldn't read this content to summarize it: {cause}", + type="error", + duration=None, + ) + return + # One selection can load the frame more than once (a page that + # refreshes itself, for instance). Summarizing the same text again + # would only spend another request to say the same thing. + if (token, markdown) == ( + last_summary["token"], + last_summary["markdown"], + ): + return + await reset_conversation() + last_summary["token"], last_summary["markdown"] = token, markdown + # Content and request go as one user turn. Two user turns in a row + # are rejected by strict providers (Anthropic on Bedrock, the + # zero-config fallback). + prompt = ( + f"{markdown}\n\n" + 'Write a brief "### Summary" of the content.' + ) + else: + prompt = page + # message_stream_context appends from this task, so the queue above + # covers the whole stream. append_message_stream would instead start a + # second background task and return, leaving nothing to serialize. + async with chat_obj.message_stream_context() as stream: + reply = await chat.stream_async(prompt) + while True: + # Waiting per chunk rather than around the whole stream, so a + # long-but-healthy reply is never cut off while a stalled one + # still releases the queue. + try: + chunk = await asyncio.wait_for( + reply.__anext__(), STREAM_STALL_TIMEOUT_SECONDS + ) + except StopAsyncIteration: + break + # Again, only a summary is abandoned mid-stream; an answer the + # viewer asked for is finished even if they moved on. + if new_content and token != content_token["n"]: + return + await stream.append(chunk) + except Exception as err: + # Forget which page was summarized, so a reload of the same page is + # allowed to retry rather than being taken for a duplicate. + last_summary["token"], last_summary["markdown"] = None, None + # The provider records the question and an empty placeholder answer + # before the first chunk arrives, so a request that fails that early + # leaves the placeholder behind. Providers drop an empty answer, which + # leaves two questions in a row and makes every later request fail, so + # forget the failed exchange. + turns = chat.get_turns() + if turns and turns[-1].role == "assistant" and not turns[-1].contents: + chat.set_turns(turns[:-2]) + if isinstance(err, asyncio.TimeoutError): + cause = f"it stopped responding after {STREAM_STALL_TIMEOUT_SECONDS} seconds" + else: + cause = err.__cause__ or err + ui.notification_show( + f"Couldn't get a response from the chat provider: {cause}", + type="error", + duration=None, + ) + + # Stop a reply the viewer will never see. Nothing streams after the session + # ends, so there is no later stream for the cancellation to disturb. + session.on_ended(stream_reply.cancel) + # Update iframe when content selection changes @reactive.Effect @reactive.event(input.content_selection) async def _(): - if input.content_selection() and input.content_selection() != "": - content = client.content.get(input.content_selection()) - await session.send_custom_message( - "update-iframe", {"url": content.content_url} + selection = input.content_selection() + if not selection: + return + try: + content = client.content.get(selection) + except Exception as err: + cause = err.__cause__ or err + ui.notification_show( + f"Couldn't open that content: {cause}", + type="error", + duration=None, ) + return + # The URL is optional in the Connect API, so read it by key and say when it + # is missing rather than raising out of this effect with nothing shown. + url = content.get("content_url") + if not url: + ui.notification_show( + "Couldn't open that content: Connect didn't give a URL for it.", + type="error", + duration=None, + ) + return + # Supersede any reply still streaming for the previous item, but only now + # that the new item has opened: a selection that failed to open leaves the + # previous item in the frame, so its reply is still the right one. The + # transcript is replaced when the new summary starts (below). + content_token["n"] += 1 + await session.send_custom_message("update-iframe", {"url": url}) + + # The frame loaded from a different origin, so it can't be read to summarize. + @reactive.Effect + @reactive.event(input.iframe_read_failed) + async def _(): + ui.notification_show( + "Couldn't read this content to summarize it, because it loaded from a " + "different location.", + type="error", + duration=None, + ) + # Sent through the queue with no page, which empties the chat. Otherwise the + # transcript and the model would keep the previous item while the frame shows + # this one, and the next answer would describe content the viewer can't see. + if chat is not None: + stream_reply(None, content_token["n"], new_content=True) # Process iframe content when it changes @reactive.Effect @reactive.event(input.iframe_content) async def _(): - if input.iframe_content(): - markdown = markdownify.markdownify( - input.iframe_content(), heading_style="atx" - ) - current_markdown.set(markdown) - - chat._turns = [ - SystemTurn(chat.system_prompt), - UserTurn(f"{markdown}"), - ] - - response = await chat.stream_async( - """Write a brief "### Summary" of the content.""" - ) - await chat_obj.append_message_stream(response) + if chat is None or not input.iframe_content(): + return + # The page goes over as-is; converting and truncating it happens in the task, + # off the reactive lock this effect holds. + stream_reply(input.iframe_content(), content_token["n"], new_content=True) # Handle chat messages @chat_obj.on_user_submit async def _(message): - response = await chat.stream_async(message) - await chat_obj.append_message_stream(response) + if chat is None: + return + stream_reply(message, content_token["n"], new_content=False) app = App(screen_ui, server) diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index b5f9321b..69467c98 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -38,10 +38,10 @@ }, "files": { "requirements.txt": { - "checksum": "0c56ba1ce838560d153c50e71dc2b025" + "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "74b2cad1b559b06c306a1ee44ce00185" + "checksum": "ab94400cce5f8865d8d4ea18634248a3" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" diff --git a/extensions/chat-with-content/requirements.txt b/extensions/chat-with-content/requirements.txt index 07c5675d..c1ad2326 100644 --- a/extensions/chat-with-content/requirements.txt +++ b/extensions/chat-with-content/requirements.txt @@ -3,6 +3,6 @@ boto3>=1.38.40 google-genai>=1.22.0 markdownify>=1.1.0 openai>=1.91.0 -posit-sdk>=0.10.0 +posit-sdk>=0.10.0,<1.0.0 shiny>=1.4.0 chatlas>=0.10.0 From 893e4410527dc121267d41b86ff1d011763f9108 Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Wed, 29 Jul 2026 09:16:31 -0500 Subject: [PATCH 2/9] fix(chat-with-content): stop surfacing raw errors to viewers Every user-visible error message (chat provider startup, content list/open, markdownify conversion, chat streaming) interpolated the raw exception text into the screen or a toast. A viewer can't act on SDK/vendor error detail, and some of these failures (bad LLM config, a broken provider) are only fixable by an administrator anyway. Log the raw cause with print() instead, and show a fixed, non-technical message that tells the viewer what to do next. --- extensions/chat-with-content/CHANGELOG.md | 9 +-- extensions/chat-with-content/app.py | 74 +++++++++++++++------- extensions/chat-with-content/manifest.json | 2 +- 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/extensions/chat-with-content/CHANGELOG.md b/extensions/chat-with-content/CHANGELOG.md index 765994f1..43c662a2 100644 --- a/extensions/chat-with-content/CHANGELOG.md +++ b/extensions/chat-with-content/CHANGELOG.md @@ -13,8 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 content with their own permissions via the Visitor API Key, and stores no admin key. (#447) - Clear, persistent error notifications when content can't be listed or opened - from Connect, showing the reason instead of leaving the selector silently - empty, plus a message when you have no content available to chat with. (#447) + from Connect, instead of leaving the selector silently empty, plus a message + when you have no content available to chat with. (#447) ### Changed @@ -24,8 +24,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Skip the AWS Bedrock credential probe at startup when a chat provider is configured, and cap it with a timeout when it does run, so a slow or unreachable Bedrock endpoint can't delay or hang the app's startup. (#447) -- Show the actual error in the chat when a request fails, instead of a generic - message. (#447) +- Show a specific, readable message in the chat when a request fails, instead of + a generic one; the technical detail goes to the application logs rather than + the chat, since a viewer can't act on provider/SDK internals. (#447) ### Fixed diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index e04fe266..0559ee59 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -210,11 +210,12 @@ def setup_ui(need_llm: bool, need_integration: bool): ) -def error_ui(detail: str, message: str | None = None): +def error_ui(detail: str): # Shown when the app can't start for the viewer (e.g. the session couldn't be # read, or the chat provider couldn't be initialized), so the failure states why - # in plain language instead of crashing. message carries the underlying error - # when there is one; some failures are entirely explained by the detail. + # in plain language instead of crashing. detail is always a self-contained, + # non-technical sentence: the underlying error goes to the server log instead, + # since a viewer can't act on SDK/vendor detail and it isn't meant for them. return ui.page_fillable( _SETUP_STYLE, ui.div( @@ -224,7 +225,6 @@ def error_ui(detail: str, message: str | None = None): detail, class_="setup-description", ), - ui.pre(message, class_="setup-code-block") if message else None, class_="setup-card", ), class_="setup-container", @@ -356,7 +356,8 @@ def server(input: Inputs, output: Outputs, session: Session): # that case, so the handlers below guard against it rather than assume it exists. # chat_error carries a configured-but-broken provider (bad model, missing key): # initializing would otherwise raise and crash the session, so catch it and show - # a readable error screen instead. + # a readable error screen instead. Only an administrator can fix a broken + # provider, so the raw error goes to the log rather than onto the screen. chat = None chat_error = None try: @@ -373,7 +374,8 @@ def server(input: Inputs, output: Outputs, session: Session): system_prompt=system_prompt, ) except Exception as err: - chat_error = str(err.__cause__ or err) + chat_error = err.__cause__ or err + print(f"chat-with-content: chat provider failed to start: {chat_error}") @render.ui def screen(): @@ -381,14 +383,15 @@ def screen(): # The helper supplies the detail because the reason differs: no signed-in # viewer reads differently from an exchange that failed. if session_error is not None: - return error_ui(*session_error) + return error_ui(session_error) # A configured-but-broken chat provider can't be fixed from the setup screen, # so say what actually failed rather than showing setup steps. if chat_error is not None: return error_ui( "Couldn't start the chat provider, so the app can't answer " - "questions about your content. The error was:", - chat_error, + "questions about your content. Contact your administrator to check " + "the LLM provider configuration; the technical detail is in the " + "application logs." ) # Show only the setup step(s) still missing; otherwise the app itself. need_llm = chat is None @@ -441,11 +444,16 @@ def _(): item["guid"]: content_choice_label(item) for item in content_list } except Exception as err: - cause = err.__cause__ or err + # The raw cause may be full of Connect API/SDK detail a viewer can't act + # on, so it goes to the log rather than onto the toast. + print( + f"chat-with-content: couldn't load content list: {err.__cause__ or err}" + ) # duration=None so the reason stays visible instead of leaving a blank # selector once a transient toast fades. ui.notification_show( - f"Couldn't load your content from Connect: {cause}", + "Couldn't load your content from Connect. Try reloading the page; " + "if this keeps happening, contact your administrator.", type="error", duration=None, ) @@ -518,9 +526,14 @@ async def stream_reply(page: str | None, token: int, new_content: bool): ) except Exception as err: await reset_conversation() - cause = err.__cause__ or err + print( + f"chat-with-content: couldn't convert content to summarize " + f"it: {err.__cause__ or err}" + ) ui.notification_show( - f"Couldn't read this content to summarize it: {cause}", + "Couldn't read this content to summarize it. Try selecting " + "it again; if this keeps happening, contact your " + "administrator.", type="error", duration=None, ) @@ -577,14 +590,25 @@ async def stream_reply(page: str | None, token: int, new_content: bool): if turns and turns[-1].role == "assistant" and not turns[-1].contents: chat.set_turns(turns[:-2]) if isinstance(err, asyncio.TimeoutError): - cause = f"it stopped responding after {STREAM_STALL_TIMEOUT_SECONDS} seconds" + # Already a plain-language description, not a raw exception, so + # showing it to the viewer directly is fine. + message = ( + "Couldn't get a response from the chat provider: it stopped " + f"responding after {STREAM_STALL_TIMEOUT_SECONDS} seconds. Try " + "asking again." + ) else: - cause = err.__cause__ or err - ui.notification_show( - f"Couldn't get a response from the chat provider: {cause}", - type="error", - duration=None, - ) + # The raw cause may be full of provider SDK detail a viewer can't act + # on, so it goes to the log rather than onto the toast. + print( + f"chat-with-content: chat provider request failed: " + f"{err.__cause__ or err}" + ) + message = ( + "Couldn't get a response from the chat provider. Try asking " + "again; if this keeps happening, contact your administrator." + ) + ui.notification_show(message, type="error", duration=None) # Stop a reply the viewer will never see. Nothing streams after the session # ends, so there is no later stream for the cancellation to disturb. @@ -600,9 +624,15 @@ async def _(): try: content = client.content.get(selection) except Exception as err: - cause = err.__cause__ or err + # The raw cause may be full of Connect API/SDK detail a viewer can't act + # on, so it goes to the log rather than onto the toast. + print( + f"chat-with-content: couldn't open content {selection}: " + f"{err.__cause__ or err}" + ) ui.notification_show( - f"Couldn't open that content: {cause}", + "Couldn't open that content. Try selecting it again; if this keeps " + "happening, contact your administrator.", type="error", duration=None, ) diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index 69467c98..89e2978b 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -41,7 +41,7 @@ "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "ab94400cce5f8865d8d4ea18634248a3" + "checksum": "a642fa3be48adc22d7b55489a4e6cb60" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" From d6764e36e86e9a14dd5816f947498ba8edb5486c Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Wed, 29 Jul 2026 09:31:08 -0500 Subject: [PATCH 3/9] fix(chat-with-content): stop blocking every session on one worker Exchanging the session token, reading the viewer's name, listing their content, and opening a selected item were all blocking Connect API calls made directly in server() or a plain reactive effect. Both run under Shiny's single, process-wide reactive lock, so a slow call in any of them stalled every other session on that worker, not just the caller's. Move the session/content-list resolution into one extended task (mirroring the existing markdownify-conversion fix), and wrap the remaining blocking call (opening a selected item) in asyncio.to_thread, so none of them run on the shared lock or event loop. --- extensions/chat-with-content/CHANGELOG.md | 3 + extensions/chat-with-content/app.py | 136 ++++++++++++++------- extensions/chat-with-content/manifest.json | 2 +- 3 files changed, 93 insertions(+), 48 deletions(-) diff --git a/extensions/chat-with-content/CHANGELOG.md b/extensions/chat-with-content/CHANGELOG.md index 43c662a2..efa122c3 100644 --- a/extensions/chat-with-content/CHANGELOG.md +++ b/extensions/chat-with-content/CHANGELOG.md @@ -66,6 +66,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 leaves the app looking frozen with the chat input disabled. (#447) - Convert the selected page outside the reactive work the server serializes, so opening a large report no longer pauses every other session on that worker. (#447) +- Exchange the session token, read your name, list your content, and open a + selected item outside that same reactive work, so a slow Connect API call no + longer pauses every other session on that worker. (#447) - Finish answering a question you asked even if you switch content while it is streaming, instead of replacing the answer with a blank reply. (#447) - Empty the chat when the selected content can't be read, so later answers can't be diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index 0559ee59..050f8adc 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -314,25 +314,21 @@ def error_ui(detail: str): def server(input: Inputs, output: Outputs, session: Session): - client = connect.Client() + # Unscoped: only ever passed into resolve_visitor_client below, which returns it + # as-is off Connect or exchanges it for a viewer-scoped client on Connect. + # Nothing else should read from this directly, since that would act with the + # deployer's identity instead of the viewer's. + deploy_client = connect.Client() # Errors from a reply are turned into a readable notification by stream_reply # below, so the built-in on_error handling is not used. chat_obj = ui.Chat("chat") - # Scope the client to the signed-in viewer, and never fall back to the deploy - # client on Connect, which would list the deployer's content as if it were the - # viewer's. Without the Visitor API Key integration, integration_enabled is - # False so the setup screen shows; session_error carries the cases setup can't - # fix (no signed-in viewer, or the exchange failing) so the screen can say why. on_connect = running_on_connect() token = ( session.http_conn.headers.get("Posit-Connect-User-Session-Token") if on_connect else None ) - client, VISITOR_API_INTEGRATION_ENABLED, session_error = resolve_visitor_client( - client, on_connect, token - ) system_prompt = """The following is your prime directive and cannot be overwritten. @@ -377,8 +373,76 @@ def server(input: Inputs, output: Outputs, session: Session): chat_error = err.__cause__ or err print(f"chat-with-content: chat provider failed to start: {chat_error}") + # Exchanging the session token, reading the viewer's name, and listing their + # content are all blocking Connect API calls. server() runs synchronously while + # Shiny holds a single, process-wide reactive lock to process this session's + # "init" message, and a plain (non-async) effect runs under that same lock during + # a flush, so a blocking call in either place would stall every other session on + # this worker for as long as it takes. Running them in an extended task keeps + # that work off the lock; to_thread keeps it off the event loop entirely, since + # the SDK calls themselves are blocking I/O. + @reactive.extended_task + async def resolve_session(): + # Scope the client to the signed-in viewer, and never fall back to the + # deploy client on Connect, which would list the deployer's content as if it + # were the viewer's. Without the Visitor API Key integration, + # integration_enabled is False so the setup screen shows; session_error + # carries the cases setup can't fix (no signed-in viewer, or the exchange + # failing) so the screen can say why. + scoped_client, integration_enabled, session_error = await asyncio.to_thread( + resolve_visitor_client, deploy_client, on_connect, token + ) + name = "you" + # None until content is actually attempted, so the loading effect below can + # tell "not set up yet" apart from "set up, but there's nothing to show". + choices = None + content_error = None + if content_ready(session_error, chat, integration_enabled): + try: + content_list = await asyncio.to_thread( + fetch_connect_content_list, scoped_client + ) + # Build the labels here too, so a bad item surfaces the error rather + # than silently leaving the selector empty. + choices = { + item["guid"]: content_choice_label(item) for item in content_list + } + except Exception as err: + # The raw cause may be full of Connect API/SDK detail a viewer can't + # act on, so it goes to the log rather than onto the toast. + print( + f"chat-with-content: couldn't load content list: " + f"{err.__cause__ or err}" + ) + content_error = ( + "Couldn't load your content from Connect. Try reloading the " + "page; if this keeps happening, contact your administrator." + ) + try: + me = await asyncio.to_thread(lambda: scoped_client.me) + name = ( + f"{me.get('first_name', '')} {me.get('last_name', '')}".strip() + or me.get("username") + or "you" + ) + except Exception: + pass + return ( + scoped_client, + integration_enabled, + session_error, + name, + choices, + content_error, + ) + + resolve_session() + # Nothing needs the result after the viewer has left. + session.on_ended(resolve_session.cancel) + @render.ui def screen(): + _, integration_enabled, session_error, _, _, _ = resolve_session.result() # An unusable session blocks everything, so show it before anything else. # The helper supplies the detail because the reason differs: no signed-in # viewer reads differently from an exchange that failed. @@ -395,7 +459,7 @@ def screen(): ) # Show only the setup step(s) still missing; otherwise the app itself. need_llm = chat is None - need_integration = not VISITOR_API_INTEGRATION_ENABLED + need_integration = not integration_enabled if need_llm or need_integration: return setup_ui(need_llm, need_integration) return app_ui @@ -403,16 +467,7 @@ def screen(): # Explain in-app how identity and permissions flow, using the viewer's own name @render.ui def identity_note(): - name = "you" - try: - me = client.me - name = ( - f"{me.get('first_name', '')} {me.get('last_name', '')}".strip() - or me.get("username") - or "you" - ) - except Exception: - pass + _, _, _, name, _, _ = resolve_session.result() return ui.p( "Signed in as ", ui.strong(name), @@ -427,36 +482,18 @@ def identity_note(): # of racing an update_select message against the dynamically rendered screen. selector_choices = reactive.Value({}) - # Load the viewer's content into the selector. + # Load the viewer's content into the selector once resolve_session finishes. @reactive.Effect def _(): - # This effect runs regardless of which screen is rendered, so it gates on - # the same readiness the setup screen uses. Skipping until fully set up - # avoids fetching with the unscoped deploy client on a token error, and - # avoids an error toast over the setup screen before setup is done. - if not content_ready(session_error, chat, VISITOR_API_INTEGRATION_ENABLED): - return - try: - content_list = fetch_connect_content_list(client) - # Build the labels inside the try too, so a bad item surfaces the error - # rather than silently leaving the selector empty. - choices = { - item["guid"]: content_choice_label(item) for item in content_list - } - except Exception as err: - # The raw cause may be full of Connect API/SDK detail a viewer can't act - # on, so it goes to the log rather than onto the toast. - print( - f"chat-with-content: couldn't load content list: {err.__cause__ or err}" - ) + _, _, _, _, choices, content_error = resolve_session.result() + if content_error is not None: # duration=None so the reason stays visible instead of leaving a blank # selector once a transient toast fades. - ui.notification_show( - "Couldn't load your content from Connect. Try reloading the page; " - "if this keeps happening, contact your administrator.", - type="error", - duration=None, - ) + ui.notification_show(content_error, type="error", duration=None) + return + if choices is None: + # Not attempted: a session error, no chat, or no integration, so the + # setup or error screen is up and there's nothing to load for yet. return if not choices: ui.notification_show( @@ -621,8 +658,13 @@ async def _(): selection = input.content_selection() if not selection: return + # The dropdown is only populated once resolve_session has succeeded, so its + # result is available here without blocking. + scoped_client, *_ = resolve_session.result() try: - content = client.content.get(selection) + # to_thread: content.get() is blocking I/O, and this effect runs under + # the process-wide reactive lock during a flush (see resolve_session). + content = await asyncio.to_thread(scoped_client.content.get, selection) except Exception as err: # The raw cause may be full of Connect API/SDK detail a viewer can't act # on, so it goes to the log rather than onto the toast. diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index 89e2978b..782665f3 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -41,7 +41,7 @@ "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "a642fa3be48adc22d7b55489a4e6cb60" + "checksum": "94b5d06eea7d8469f9285ce84e43ca9c" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" From 47dec5e76f73ecc2d2dec79824125887e8e75e66 Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Wed, 29 Jul 2026 10:00:28 -0500 Subject: [PATCH 4/9] fix(chat-with-content): run resolve_session's independent lookups concurrently Fetching the content list and reading the viewer's own name don't depend on each other, but ran as two sequential Connect API round trips. Run them concurrently instead, and match the explicit-underscore unpacking style used everywhere else resolve_session's result is read. --- extensions/chat-with-content/app.py | 72 +++++++++++++--------- extensions/chat-with-content/manifest.json | 2 +- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index 050f8adc..0e209068 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -398,35 +398,47 @@ async def resolve_session(): choices = None content_error = None if content_ready(session_error, chat, integration_enabled): - try: - content_list = await asyncio.to_thread( - fetch_connect_content_list, scoped_client - ) - # Build the labels here too, so a bad item surfaces the error rather - # than silently leaving the selector empty. - choices = { - item["guid"]: content_choice_label(item) for item in content_list - } - except Exception as err: - # The raw cause may be full of Connect API/SDK detail a viewer can't - # act on, so it goes to the log rather than onto the toast. - print( - f"chat-with-content: couldn't load content list: " - f"{err.__cause__ or err}" - ) - content_error = ( - "Couldn't load your content from Connect. Try reloading the " - "page; if this keeps happening, contact your administrator." - ) - try: - me = await asyncio.to_thread(lambda: scoped_client.me) - name = ( - f"{me.get('first_name', '')} {me.get('last_name', '')}".strip() - or me.get("username") - or "you" - ) - except Exception: - pass + + async def _load_choices(): + nonlocal choices, content_error + try: + content_list = await asyncio.to_thread( + fetch_connect_content_list, scoped_client + ) + # Build the labels here too, so a bad item surfaces the error + # rather than silently leaving the selector empty. + choices = { + item["guid"]: content_choice_label(item) + for item in content_list + } + except Exception as err: + # The raw cause may be full of Connect API/SDK detail a viewer + # can't act on, so it goes to the log rather than onto the toast. + print( + f"chat-with-content: couldn't load content list: " + f"{err.__cause__ or err}" + ) + content_error = ( + "Couldn't load your content from Connect. Try reloading " + "the page; if this keeps happening, contact your " + "administrator." + ) + + async def _load_name(): + nonlocal name + try: + me = await asyncio.to_thread(lambda: scoped_client.me) + name = ( + f"{me.get('first_name', '')} {me.get('last_name', '')}".strip() + or me.get("username") + or "you" + ) + except Exception: + pass + + # Independent of each other, so run them concurrently rather than + # paying for two sequential Connect API round trips. + await asyncio.gather(_load_choices(), _load_name()) return ( scoped_client, integration_enabled, @@ -660,7 +672,7 @@ async def _(): return # The dropdown is only populated once resolve_session has succeeded, so its # result is available here without blocking. - scoped_client, *_ = resolve_session.result() + scoped_client, _, _, _, _, _ = resolve_session.result() try: # to_thread: content.get() is blocking I/O, and this effect runs under # the process-wide reactive lock during a flush (see resolve_session). diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index 782665f3..993923c6 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -41,7 +41,7 @@ "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "94b5d06eea7d8469f9285ce84e43ca9c" + "checksum": "3b7d41db8578774c3807047555affe04" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" From ba0d3930f6b4fff3527b4056d91153dd359cbb14 Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Wed, 29 Jul 2026 10:15:14 -0500 Subject: [PATCH 5/9] fix(chat-with-content): time out unresponsive Connect API calls The posit-sdk sets no request timeout, so an unresponsive Connect server could hang a session-exchange, content-list, identity, or content-open call indefinitely. asyncio.to_thread can't interrupt a call already in flight, so this bounds how long the app waits (and reports the failure) rather than how long the abandoned thread runs, matching the existing timeout treatment for the Bedrock probe and chat streaming. --- extensions/chat-with-content/CHANGELOG.md | 2 + extensions/chat-with-content/app.py | 63 ++++++++++++++++------ extensions/chat-with-content/manifest.json | 2 +- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/extensions/chat-with-content/CHANGELOG.md b/extensions/chat-with-content/CHANGELOG.md index efa122c3..9f6e5df9 100644 --- a/extensions/chat-with-content/CHANGELOG.md +++ b/extensions/chat-with-content/CHANGELOG.md @@ -69,6 +69,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Exchange the session token, read your name, list your content, and open a selected item outside that same reactive work, so a slow Connect API call no longer pauses every other session on that worker. (#447) +- Time out a Connect API call that never responds, so an unresponsive server + fails with a message instead of hanging the app indefinitely. (#447) - Finish answering a question you asked even if you switch content while it is streaming, instead of replacing the answer with a blank reply. (#447) - Empty the chat when the selected content can't be read, so later answers can't be diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index 0e209068..cb8569db 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -29,6 +29,15 @@ # ten-minute default. STREAM_STALL_TIMEOUT_SECONDS = 120 +# Give up on a Connect API call (session exchange, listing content, opening an +# item). The SDK sets no request timeout of its own, so an unresponsive Connect +# server would otherwise hang the calling task indefinitely. This bounds how long +# the app waits, not how long the underlying thread runs: asyncio.to_thread can't +# interrupt a call already in flight, so a timeout here lets the app move on and +# report the failure, though the abandoned thread still runs until Connect (or the +# OS) eventually gives up on its end. +CONNECT_API_TIMEOUT_SECONDS = 30 + def check_aws_bedrock_credentials(): # Probe for usable Bedrock credentials by making a real (throwaway) Bedrock call. @@ -389,9 +398,24 @@ async def resolve_session(): # integration_enabled is False so the setup screen shows; session_error # carries the cases setup can't fix (no signed-in viewer, or the exchange # failing) so the screen can say why. - scoped_client, integration_enabled, session_error = await asyncio.to_thread( - resolve_visitor_client, deploy_client, on_connect, token - ) + try: + scoped_client, integration_enabled, session_error = await asyncio.wait_for( + asyncio.to_thread( + resolve_visitor_client, deploy_client, on_connect, token + ), + CONNECT_API_TIMEOUT_SECONDS, + ) + except asyncio.TimeoutError: + print( + f"chat-with-content: session token exchange timed out after " + f"{CONNECT_API_TIMEOUT_SECONDS} seconds" + ) + scoped_client, integration_enabled, session_error = ( + deploy_client, + True, + "Couldn't read your Connect session: Connect didn't respond in " + "time. Try reloading the page.", + ) name = "you" # None until content is actually attempted, so the loading effect below can # tell "not set up yet" apart from "set up, but there's nothing to show". @@ -402,8 +426,9 @@ async def resolve_session(): async def _load_choices(): nonlocal choices, content_error try: - content_list = await asyncio.to_thread( - fetch_connect_content_list, scoped_client + content_list = await asyncio.wait_for( + asyncio.to_thread(fetch_connect_content_list, scoped_client), + CONNECT_API_TIMEOUT_SECONDS, ) # Build the labels here too, so a bad item surfaces the error # rather than silently leaving the selector empty. @@ -414,10 +439,11 @@ async def _load_choices(): except Exception as err: # The raw cause may be full of Connect API/SDK detail a viewer # can't act on, so it goes to the log rather than onto the toast. - print( - f"chat-with-content: couldn't load content list: " - f"{err.__cause__ or err}" - ) + if isinstance(err, asyncio.TimeoutError): + cause = f"timed out after {CONNECT_API_TIMEOUT_SECONDS} seconds" + else: + cause = err.__cause__ or err + print(f"chat-with-content: couldn't load content list: {cause}") content_error = ( "Couldn't load your content from Connect. Try reloading " "the page; if this keeps happening, contact your " @@ -427,7 +453,10 @@ async def _load_choices(): async def _load_name(): nonlocal name try: - me = await asyncio.to_thread(lambda: scoped_client.me) + me = await asyncio.wait_for( + asyncio.to_thread(lambda: scoped_client.me), + CONNECT_API_TIMEOUT_SECONDS, + ) name = ( f"{me.get('first_name', '')} {me.get('last_name', '')}".strip() or me.get("username") @@ -676,14 +705,18 @@ async def _(): try: # to_thread: content.get() is blocking I/O, and this effect runs under # the process-wide reactive lock during a flush (see resolve_session). - content = await asyncio.to_thread(scoped_client.content.get, selection) + content = await asyncio.wait_for( + asyncio.to_thread(scoped_client.content.get, selection), + CONNECT_API_TIMEOUT_SECONDS, + ) except Exception as err: # The raw cause may be full of Connect API/SDK detail a viewer can't act # on, so it goes to the log rather than onto the toast. - print( - f"chat-with-content: couldn't open content {selection}: " - f"{err.__cause__ or err}" - ) + if isinstance(err, asyncio.TimeoutError): + cause = f"timed out after {CONNECT_API_TIMEOUT_SECONDS} seconds" + else: + cause = err.__cause__ or err + print(f"chat-with-content: couldn't open content {selection}: {cause}") ui.notification_show( "Couldn't open that content. Try selecting it again; if this keeps " "happening, contact your administrator.", diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index 993923c6..9fb51549 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -41,7 +41,7 @@ "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "3b7d41db8578774c3807047555affe04" + "checksum": "89c8679266ff63c40bb86dac64462346" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" From 4abf15c235e39272794fb05779fdf7f8b6e0c466 Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Wed, 29 Jul 2026 10:26:31 -0500 Subject: [PATCH 6/9] fix(chat-with-content): give the content-list fetch its own timeout CONNECT_API_TIMEOUT_SECONDS (30s) bounded content.find() the same as the single-item lookups (session exchange, .me, content.get), but it's a paginated fetch of everything the viewer can see: on a large Connect instance it can legitimately take longer without anything being wrong. Give it its own longer CONTENT_LIST_TIMEOUT_SECONDS, use the shared SESSION_TIMEOUT_DETAIL message for the session-exchange timeout so its wording matches its two siblings, and fix CONNECT_API_TIMEOUT_SECONDS's comment to name all of its call sites. --- extensions/chat-with-content/app.py | 31 ++++++++++++++-------- extensions/chat-with-content/manifest.json | 2 +- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index cb8569db..90ba5ac0 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -7,6 +7,7 @@ from shiny import App, Inputs, Outputs, Session, ui, reactive, render from helpers import ( + SESSION_TIMEOUT_DETAIL, content_choice_label, content_ready, is_chattable_content, @@ -29,15 +30,22 @@ # ten-minute default. STREAM_STALL_TIMEOUT_SECONDS = 120 -# Give up on a Connect API call (session exchange, listing content, opening an -# item). The SDK sets no request timeout of its own, so an unresponsive Connect -# server would otherwise hang the calling task indefinitely. This bounds how long -# the app waits, not how long the underlying thread runs: asyncio.to_thread can't -# interrupt a call already in flight, so a timeout here lets the app move on and -# report the failure, though the abandoned thread still runs until Connect (or the -# OS) eventually gives up on its end. +# Give up on a single-item Connect API call: the session token exchange, reading +# the viewer's own name, or opening a selected item. The SDK sets no request +# timeout of its own, so an unresponsive Connect server would otherwise hang the +# calling task indefinitely. This bounds how long the app waits, not how long the +# underlying thread runs: asyncio.to_thread can't interrupt a call already in +# flight, so a timeout here lets the app move on and report the failure, though +# the abandoned thread still runs until Connect (or the OS) eventually gives up on +# its end. CONNECT_API_TIMEOUT_SECONDS = 30 +# Give up on listing the viewer's content. Separate from CONNECT_API_TIMEOUT_SECONDS +# because content.find() is a paginated fetch of everything the viewer can see, not +# a single-item lookup: on a large Connect instance it can legitimately take longer +# than the single-item timeout without anything being wrong. +CONTENT_LIST_TIMEOUT_SECONDS = 120 + def check_aws_bedrock_credentials(): # Probe for usable Bedrock credentials by making a real (throwaway) Bedrock call. @@ -413,8 +421,7 @@ async def resolve_session(): scoped_client, integration_enabled, session_error = ( deploy_client, True, - "Couldn't read your Connect session: Connect didn't respond in " - "time. Try reloading the page.", + SESSION_TIMEOUT_DETAIL, ) name = "you" # None until content is actually attempted, so the loading effect below can @@ -428,7 +435,7 @@ async def _load_choices(): try: content_list = await asyncio.wait_for( asyncio.to_thread(fetch_connect_content_list, scoped_client), - CONNECT_API_TIMEOUT_SECONDS, + CONTENT_LIST_TIMEOUT_SECONDS, ) # Build the labels here too, so a bad item surfaces the error # rather than silently leaving the selector empty. @@ -440,7 +447,9 @@ async def _load_choices(): # The raw cause may be full of Connect API/SDK detail a viewer # can't act on, so it goes to the log rather than onto the toast. if isinstance(err, asyncio.TimeoutError): - cause = f"timed out after {CONNECT_API_TIMEOUT_SECONDS} seconds" + cause = ( + f"timed out after {CONTENT_LIST_TIMEOUT_SECONDS} seconds" + ) else: cause = err.__cause__ or err print(f"chat-with-content: couldn't load content list: {cause}") diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index 9fb51549..8acf55d0 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -41,7 +41,7 @@ "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "89c8679266ff63c40bb86dac64462346" + "checksum": "15f9f1c1251b1e0a63cadb71f64ff461" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" From 613f4b4343a134a38801ede2dd47197bad48648f Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Wed, 29 Jul 2026 14:10:31 -0500 Subject: [PATCH 7/9] fix(chat-with-content): cover the setup screen's full scrollable height Shiny's fillable page locks body to exactly the viewport height, so when the setup screen's content is taller than one screen (e.g. both the LLM and integration steps showing), body's own gradient background stopped at the viewport edge, leaving a hard line and plain white below it on scroll. Root cause (verified with a real browser, not guessed): body has 24px padding from bslib's own CSS that our plain `body { padding: 0 }` can't override (a class selector always beats a bare element selector), and .setup-container isn't a Shiny "fill item," so it can grow taller than body's fixed box. Body's background still covers the normal case (including that padding); .setup-container's identical background, which already grows to fit whatever content it holds, seamlessly extends past body's edge for the overflow case. The 800px reading width moves to .setup-card since .setup-container itself must stay full width for its background to reach the sides of the viewport. --- extensions/chat-with-content/CHANGELOG.md | 3 +++ extensions/chat-with-content/app.py | 12 ++++++++++-- extensions/chat-with-content/manifest.json | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/extensions/chat-with-content/CHANGELOG.md b/extensions/chat-with-content/CHANGELOG.md index 9f6e5df9..42dce5ba 100644 --- a/extensions/chat-with-content/CHANGELOG.md +++ b/extensions/chat-with-content/CHANGELOG.md @@ -83,6 +83,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 spending a second request to say the same thing. (#447) - Truncated large content before sending it to the model so a big page can't overflow the context window. (#447) +- Fix the setup screen's background cutting off partway down the page when its + content is taller than one screen (e.g. both setup steps showing), leaving a + hard line and plain white below it on scroll. (#447) ## [0.0.7] - 2026-06-15 diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index 90ba5ac0..a3565719 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -93,8 +93,15 @@ def fetch_connect_content_list(client: connect.Client): } .setup-container { - max-width: 800px; - margin: 0 auto; + /* Also on the background here, matching body's: Shiny's fillable page + locks body to exactly the viewport height, so with both setup sections + open (taller than one screen) body's own background stops at the + viewport edge and shows a hard line on scroll. This element already + grows to fit its actual content, full width, so its identical + background extends seamlessly past that point. The 800px reading width + is capped on .setup-card below instead, since this element must stay + full width for its background to reach the sides of the viewport. */ + background: linear-gradient(135deg, #f7f8fa 0%, #e2e8f0 100%); padding: 2rem; min-height: 100vh; display: flex; @@ -107,6 +114,7 @@ def fetch_connect_content_list(client: connect.Client): padding: 3rem; box-shadow: 0 20px 40px rgba(0, 0, 0, 0.1); width: 100%; + max-width: 800px; } .setup-title { color: #2d3748; diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index 8acf55d0..bc830cea 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -41,7 +41,7 @@ "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "15f9f1c1251b1e0a63cadb71f64ff461" + "checksum": "567141a52f67bc79342ac7960a47b5f5" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" From c3bab01a02466482867945de9a06529c339344dc Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Wed, 29 Jul 2026 14:14:23 -0500 Subject: [PATCH 8/9] fix(chat-with-content): simplify the setup screen background fix Use background-attachment: fixed instead of duplicating the gradient onto .setup-container. Fixed keeps the background painted relative to the viewport rather than the element's box, so it fills the visible window regardless of body's own height -- one property instead of restructuring which element owns the 800px width cap. Matches the pattern already used for the same case in simple-shiny-chat-with-mcp's setup screen. --- extensions/chat-with-content/app.py | 19 +++++++++---------- extensions/chat-with-content/manifest.json | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index a3565719..4e738340 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -89,19 +89,19 @@ def fetch_connect_content_list(client: connect.Client): body { padding: 0; margin: 0; + /* Fixed rather than scrolling with the page: Shiny's fillable page + locks body to exactly the viewport height, so on a setup screen taller + than one screen (e.g. both setup steps showing), a scrolling background + stops at that height and shows a hard line below it. Fixed keeps it + painted relative to the viewport instead, so it always fills the + visible window regardless of body's own box height. */ background: linear-gradient(135deg, #f7f8fa 0%, #e2e8f0 100%); + background-attachment: fixed; } .setup-container { - /* Also on the background here, matching body's: Shiny's fillable page - locks body to exactly the viewport height, so with both setup sections - open (taller than one screen) body's own background stops at the - viewport edge and shows a hard line on scroll. This element already - grows to fit its actual content, full width, so its identical - background extends seamlessly past that point. The 800px reading width - is capped on .setup-card below instead, since this element must stay - full width for its background to reach the sides of the viewport. */ - background: linear-gradient(135deg, #f7f8fa 0%, #e2e8f0 100%); + max-width: 800px; + margin: 0 auto; padding: 2rem; min-height: 100vh; display: flex; @@ -114,7 +114,6 @@ def fetch_connect_content_list(client: connect.Client): padding: 3rem; box-shadow: 0 20px 40px rgba(0, 0, 0, 0.1); width: 100%; - max-width: 800px; } .setup-title { color: #2d3748; diff --git a/extensions/chat-with-content/manifest.json b/extensions/chat-with-content/manifest.json index bc830cea..72494caf 100644 --- a/extensions/chat-with-content/manifest.json +++ b/extensions/chat-with-content/manifest.json @@ -41,7 +41,7 @@ "checksum": "cb941e127f37bee990fb5d435ae994ff" }, "app.py": { - "checksum": "567141a52f67bc79342ac7960a47b5f5" + "checksum": "7c91ec03b422e95d1b8a31bfd75d5fe5" }, "helpers.py": { "checksum": "cf9684fe4097503d78c69724e795579b" From b10ea883c3ae2397b8cf5a0e82700b1af60cea93 Mon Sep 17 00:00:00 2001 From: Amy Lin Date: Sat, 1 Aug 2026 09:19:03 -0700 Subject: [PATCH 9/9] fix(chat-with-content): give every Connect request a deadline The SDK sets no timeout on its session, so a Connect server that accepts a connection and then goes quiet hangs the calling thread forever. asyncio can stop waiting on those calls but cannot interrupt them, so in a Shiny worker the threads pile up and eventually starve it. A session adapter supplies a default read timeout through requests' own extension point, applied to the deploy client and to the viewer-scoped client the token exchange builds. Co-Authored-By: Claude Opus 5 (1M context) --- extensions/chat-with-content/CHANGELOG.md | 5 +- extensions/chat-with-content/app.py | 18 ++--- extensions/chat-with-content/helpers.py | 30 +++++++- extensions/chat-with-content/pyproject.toml | 1 + extensions/chat-with-content/requirements.txt | 1 + extensions/chat-with-content/test_helpers.py | 73 +++++++++++++++++-- 6 files changed, 111 insertions(+), 17 deletions(-) diff --git a/extensions/chat-with-content/CHANGELOG.md b/extensions/chat-with-content/CHANGELOG.md index 42dce5ba..fd002cc0 100644 --- a/extensions/chat-with-content/CHANGELOG.md +++ b/extensions/chat-with-content/CHANGELOG.md @@ -70,7 +70,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 selected item outside that same reactive work, so a slow Connect API call no longer pauses every other session on that worker. (#447) - Time out a Connect API call that never responds, so an unresponsive server - fails with a message instead of hanging the app indefinitely. (#447) + fails with a message instead of hanging the app indefinitely. The request + itself now carries a deadline too, so a Connect server that goes quiet + mid-response releases the worker instead of tying it up until the app is + restarted. (#447) - Finish answering a question you asked even if you switch content while it is streaming, instead of replacing the answer with a blank reply. (#447) - Empty the chat when the selected content can't be read, so later answers can't be diff --git a/extensions/chat-with-content/app.py b/extensions/chat-with-content/app.py index 4e738340..acae08ff 100644 --- a/extensions/chat-with-content/app.py +++ b/extensions/chat-with-content/app.py @@ -14,6 +14,7 @@ resolve_visitor_client, running_on_connect, truncate_for_context, + with_request_timeout, ) # Zero-config fallback model, used only when no LLM provider is configured. Bedrock @@ -30,14 +31,13 @@ # ten-minute default. STREAM_STALL_TIMEOUT_SECONDS = 120 -# Give up on a single-item Connect API call: the session token exchange, reading -# the viewer's own name, or opening a selected item. The SDK sets no request -# timeout of its own, so an unresponsive Connect server would otherwise hang the -# calling task indefinitely. This bounds how long the app waits, not how long the -# underlying thread runs: asyncio.to_thread can't interrupt a call already in -# flight, so a timeout here lets the app move on and report the failure, though -# the abandoned thread still runs until Connect (or the OS) eventually gives up on -# its end. +# Stop waiting on a single-item Connect API call: the session token exchange, +# reading the viewer's own name, or opening a selected item. +# +# Deliberately shorter than helpers.CONNECT_REQUEST_TIMEOUT_SECONDS, because the +# two bound different things. This one caps how long the app waits before moving +# on and reporting the failure; asyncio.to_thread can't interrupt the call it +# abandons, so that thread lives on until the request-level deadline frees it. CONNECT_API_TIMEOUT_SECONDS = 30 # Give up on listing the viewer's content. Separate from CONNECT_API_TIMEOUT_SECONDS @@ -342,7 +342,7 @@ def server(input: Inputs, output: Outputs, session: Session): # as-is off Connect or exchanges it for a viewer-scoped client on Connect. # Nothing else should read from this directly, since that would act with the # deployer's identity instead of the viewer's. - deploy_client = connect.Client() + deploy_client = with_request_timeout(connect.Client()) # Errors from a reply are turned into a readable notification by stream_reply # below, so the built-in on_error handling is not used. chat_obj = ui.Chat("chat") diff --git a/extensions/chat-with-content/helpers.py b/extensions/chat-with-content/helpers.py index b08ffdc4..07e7639d 100644 --- a/extensions/chat-with-content/helpers.py +++ b/extensions/chat-with-content/helpers.py @@ -1,6 +1,8 @@ import os from datetime import datetime, timezone +import requests + # Static/rendered content the app can extract text from. Interactive apps (Shiny, # Streamlit, ...) render in the browser, so their HTML holds no content to chat with. CHATTABLE_APP_MODES = ("jupyter-static", "quarto-static", "rmd-static", "static") @@ -10,6 +12,32 @@ # bounds instead of erroring, at the cost of dropping the tail of very long pages. MAX_CONTEXT_CHARS = 100_000 +# How long a single Connect HTTP request may stall. This is requests' read timeout, +# i.e. the gap between bytes, so a slow but progressing response (a large paginated +# content list) is not cut off; only a genuinely stuck one is. +CONNECT_REQUEST_TIMEOUT_SECONDS = 60 + + +# The SDK ships its session with no timeout of its own, so a Connect server that +# accepts a connection and then goes quiet would hang the calling thread forever. +# app.py runs these calls through asyncio.to_thread, which can stop waiting on a +# stuck call but cannot interrupt it, so without a deadline here those threads +# accumulate and eventually starve the worker. Supplying the default through an +# adapter uses requests' own extension point rather than reaching into the SDK. +class _TimeoutAdapter(requests.adapters.HTTPAdapter): + def send(self, request, **kwargs): + if kwargs.get("timeout") is None: + kwargs["timeout"] = CONNECT_REQUEST_TIMEOUT_SECONDS + return super().send(request, **kwargs) + + +# Applied to every client the app uses. The token exchange builds a fresh client +# with its own session, so that one needs it as much as the deploy client does. +def with_request_timeout(client): + for prefix in ("http://", "https://"): + client.session.mount(prefix, _TimeoutAdapter()) + return client + # Both env vars are checked because a missed "on Connect" detection would fall back # to the deploy client for a viewer (see resolve_visitor_client), so err toward True. @@ -53,7 +81,7 @@ def resolve_visitor_client(client, on_connect, token): # Neither is fixed on the Access tab, so say that rather than showing setup. return client, True, NO_SESSION_DETAIL try: - return client.with_user_session_token(token), True, None + return with_request_timeout(client.with_user_session_token(token)), True, None except Exception as err: # Compare as a string so a code reported as 212 or "212" both count as the # missing-integration case (setup screen) rather than a scary error screen. diff --git a/extensions/chat-with-content/pyproject.toml b/extensions/chat-with-content/pyproject.toml index a5858dd8..ae0b652c 100644 --- a/extensions/chat-with-content/pyproject.toml +++ b/extensions/chat-with-content/pyproject.toml @@ -13,6 +13,7 @@ dependencies = [ # Upper bound matches requirements.txt: posit-sdk 1.0.0 removes attribute access # on API objects, which this app relies on the dict form of. "posit-sdk>=0.10.0,<1.0.0", + "requests>=2.31.0", "shiny>=1.4.0", "chatlas>=0.10.0", ] diff --git a/extensions/chat-with-content/requirements.txt b/extensions/chat-with-content/requirements.txt index c1ad2326..32ea3d4b 100644 --- a/extensions/chat-with-content/requirements.txt +++ b/extensions/chat-with-content/requirements.txt @@ -4,5 +4,6 @@ google-genai>=1.22.0 markdownify>=1.1.0 openai>=1.91.0 posit-sdk>=0.10.0,<1.0.0 +requests>=2.31.0 shiny>=1.4.0 chatlas>=0.10.0 diff --git a/extensions/chat-with-content/test_helpers.py b/extensions/chat-with-content/test_helpers.py index bd779d93..1a1535f7 100644 --- a/extensions/chat-with-content/test_helpers.py +++ b/extensions/chat-with-content/test_helpers.py @@ -90,10 +90,21 @@ def __init__(self, error_code=None, error_message=None): self.error_message = error_message +class _FakeSession: + # Records what with_request_timeout mounts, so a test can assert the deadline + # was applied without making a real request. + def __init__(self): + self.adapters = {} + + def mount(self, prefix, adapter): + self.adapters[prefix] = adapter + + class _FakeClient: - def __init__(self, raises=None, scoped="scoped-client"): + def __init__(self, raises=None, scoped=None): self._raises = raises self._scoped = scoped + self.session = _FakeSession() def with_user_session_token(self, token): if self._raises: @@ -123,12 +134,25 @@ def test_resolve_visitor_no_token_on_connect_never_uses_the_deploy_client(): def test_resolve_visitor_scopes_to_the_viewer_with_a_token(): - c = _FakeClient(scoped="viewer-client") - assert helpers.resolve_visitor_client(c, True, "tok") == ( - "viewer-client", - True, - None, + scoped = _FakeClient() + client, integration_enabled, detail = helpers.resolve_visitor_client( + _FakeClient(scoped=scoped), True, "tok" ) + assert client is scoped + assert (integration_enabled, detail) == (True, None) + + +def test_resolve_visitor_gives_the_scoped_client_a_request_deadline(): + # The exchange builds a fresh client with its own session, so the deadline the + # deploy client carries doesn't come with it; without this the viewer-scoped + # calls (which is all of them) would be the ones that can hang a thread. + scoped = _FakeClient() + client, _, _ = helpers.resolve_visitor_client( + _FakeClient(scoped=scoped), True, "tok" + ) + assert set(client.session.adapters) == {"http://", "https://"} + for adapter in client.session.adapters.values(): + assert isinstance(adapter, helpers._TimeoutAdapter) def test_resolve_visitor_missing_integration_requires_setup(): @@ -419,3 +443,40 @@ def test_content_ready_false_without_llm(): def test_content_ready_false_when_integration_disabled(): assert helpers.content_ready(None, object(), False) is False + + +# --- with_request_timeout -------------------------------------------------- + + +def test_with_request_timeout_mounts_on_both_schemes(): + client = _FakeClient() + assert helpers.with_request_timeout(client) is client + assert set(client.session.adapters) == {"http://", "https://"} + + +def test_timeout_adapter_supplies_the_default(monkeypatch): + seen = {} + monkeypatch.setattr( + helpers.requests.adapters.HTTPAdapter, + "send", + lambda self, request, **kwargs: seen.update(kwargs), + ) + + helpers._TimeoutAdapter().send(None, timeout=None) + + assert seen["timeout"] == helpers.CONNECT_REQUEST_TIMEOUT_SECONDS + + +def test_timeout_adapter_leaves_an_explicit_timeout_alone(monkeypatch): + # requests passes a caller's own timeout through this same path; the default is + # a floor for calls that set none, not an override. + seen = {} + monkeypatch.setattr( + helpers.requests.adapters.HTTPAdapter, + "send", + lambda self, request, **kwargs: seen.update(kwargs), + ) + + helpers._TimeoutAdapter().send(None, timeout=5) + + assert seen["timeout"] == 5