mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-15 04:17:56 +00:00
feat(api): serve long-lived streams on the event loop
Move GET /api/events, the remote-device command stream and artifact downloads from Flask to Starlette routes mounted ahead of the Flask catch-all. On Flask each held an a2wsgi threadpool slot for as long as its response stayed open, and because uvicorn drops writes after a client disconnects, a closed tab never released it. - asgi_auth: one JWT/OIDC gate for Starlette routes; the chat reconnect reader uses it too - ClosingStreamingResponse closes the body iterator and releases the SSE slot or file handle even when the client leaves before the first frame - AsyncTopic liveness probe replaces the sync client's socket_timeout guard against half-open pub/sub sockets - ASYNC_REDIS_MAX_CONNECTIONS sizes the async Redis pool; every open stream holds a connection and redis-py defaults to 100
This commit is contained in:
1 parent
38773471a8
commit
54b540ea42
33 files changed
+2822
-1476
No files matched your search
@@ -3,7 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
from typing import Optional
|
||||
from typing import Mapping, Optional
|
||||
|
||||
from flask import request
|
||||
|
||||
@@ -40,18 +40,16 @@ class Principal:
|
||||
return self.agent_id is not None
|
||||
|
||||
|
||||
def resolve_principal() -> Principal:
|
||||
"""Resolve the caller to a :class:`Principal`.
|
||||
def principal_for(decoded_token: Optional[dict], api_key: Optional[str]) -> Principal:
|
||||
"""Resolve a caller from its decoded JWT or ``api_key``, independent of the web framework.
|
||||
|
||||
Priority: a decoded JWT (owner session) first, then an ``api_key`` query/form
|
||||
param (agent-scoped). An unresolvable caller yields an anonymous principal
|
||||
(both fields ``None``) that only a valid ``share_token`` can authorize.
|
||||
Priority: a decoded JWT (owner session) first, then the ``api_key``
|
||||
(agent-scoped). An unresolvable caller yields an anonymous principal (both
|
||||
fields ``None``) that only a valid ``share_token`` can authorize.
|
||||
"""
|
||||
decoded_token = getattr(request, "decoded_token", None)
|
||||
if decoded_token:
|
||||
return Principal(user_id=decoded_token.get("sub"))
|
||||
|
||||
api_key = request.args.get("api_key") or request.form.get("api_key")
|
||||
if api_key:
|
||||
with db_readonly() as conn:
|
||||
agent = AgentsRepository(conn).find_by_key(api_key)
|
||||
@@ -60,6 +58,18 @@ def resolve_principal() -> Principal:
|
||||
return Principal()
|
||||
|
||||
|
||||
def resolve_principal() -> Principal:
|
||||
"""Resolve the current Flask request's caller to a :class:`Principal`.
|
||||
|
||||
Reads the decoded JWT, then an ``api_key`` query/form param; see
|
||||
:func:`principal_for`.
|
||||
"""
|
||||
return principal_for(
|
||||
getattr(request, "decoded_token", None),
|
||||
request.args.get("api_key") or request.form.get("api_key"),
|
||||
)
|
||||
|
||||
|
||||
def _shared_row_for(conn, conversation_id, share_token):
|
||||
"""Return the shared_conversations row iff share_token grants this conversation, else None."""
|
||||
if not share_token:
|
||||
@@ -87,7 +97,12 @@ def user_can_access_conversation(
|
||||
return _shared_row_for(conn, conversation_id, share_token) is not None
|
||||
|
||||
|
||||
def authorize_artifact(conn, artifact: dict, principal: Principal) -> bool:
|
||||
def authorize_artifact(
|
||||
conn,
|
||||
artifact: dict,
|
||||
principal: Principal,
|
||||
args: Optional[Mapping[str, str]] = None,
|
||||
) -> bool:
|
||||
"""Authorize a READ of ``artifact`` for ``principal``; missing parent fails closed.
|
||||
|
||||
A low-trust agent api_key is confined to a single conversation it proves by
|
||||
@@ -96,7 +111,13 @@ def authorize_artifact(conn, artifact: dict, principal: Principal) -> bool:
|
||||
JWT owner or ``shared_with`` collaborator gets full access to every artifact of
|
||||
the parent; a share-token holder is confined to the shared ``first_n_queries``
|
||||
snapshot (an artifact whose ``message_id`` is outside it, or NULL, is denied).
|
||||
|
||||
``args`` are the request's query params; they default to the current Flask
|
||||
request's, and a native-async route passes its own.
|
||||
"""
|
||||
if args is None:
|
||||
args = request.args
|
||||
|
||||
if principal.is_agent_scoped:
|
||||
# An agent key is not the owner's session and is embedded in public widget
|
||||
# JS, so require the artifact to belong to the owner AND the request to
|
||||
@@ -104,7 +125,7 @@ def authorize_artifact(conn, artifact: dict, principal: Principal) -> bool:
|
||||
# the key alone, without that id, cannot download a known artifact.
|
||||
if str(artifact.get("user_id")) != str(principal.user_id):
|
||||
return False
|
||||
req_conv = request.args.get("conversation_id")
|
||||
req_conv = args.get("conversation_id")
|
||||
parent = artifact.get("conversation_id")
|
||||
if not req_conv or parent is None or str(parent) != str(req_conv):
|
||||
return False
|
||||
@@ -114,7 +135,7 @@ def authorize_artifact(conn, artifact: dict, principal: Principal) -> bool:
|
||||
|
||||
conversation_id = artifact.get("conversation_id")
|
||||
workflow_run_id = artifact.get("workflow_run_id")
|
||||
share_token = request.args.get("share_token")
|
||||
share_token = args.get("share_token")
|
||||
|
||||
if conversation_id is not None:
|
||||
# Owner or shared_with collaborator: full access to every artifact.
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
"""``GET /api/artifacts/{artifact_id}/download`` — an artifact version's bytes.
|
||||
|
||||
A native-async Starlette route mounted ahead of Flask in ``docsgpt/asgi.py``,
|
||||
so a large download to a slow client streams from the event loop instead of
|
||||
pinning a WSGI threadpool slot for the whole transfer. Authorization is the
|
||||
same parent-derived check the Flask artifact routes use; the database reads
|
||||
and storage calls run in worker threads.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import logging
|
||||
import re
|
||||
from functools import partial
|
||||
from typing import AsyncIterator, BinaryIO, Mapping, Optional, Tuple
|
||||
|
||||
import anyio
|
||||
from starlette.requests import Request
|
||||
from starlette.responses import JSONResponse, RedirectResponse, Response
|
||||
from starlette.routing import Route
|
||||
|
||||
from docsgpt.api.asgi_auth import authenticate, bind_log_context, json_error
|
||||
from docsgpt.api.asgi_stream import ClosingStreamingResponse
|
||||
from docsgpt.api.user.artifacts.authz import authorize_artifact, principal_for
|
||||
from docsgpt.core.settings import settings
|
||||
from docsgpt.storage.db.base_repository import looks_like_uuid
|
||||
from docsgpt.storage.db.repositories.artifacts import ArtifactsRepository
|
||||
from docsgpt.storage.db.session import db_readonly
|
||||
from docsgpt.storage.storage_creator import StorageCreator
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Presigned-URL TTL for private S3 artifact downloads (seconds).
|
||||
_PRESIGNED_URL_TTL = 300
|
||||
|
||||
_ARTIFACT_URL_ENVELOPE_MIME = "application/vnd.docsgpt.artifact-url+json"
|
||||
|
||||
_CHUNK_SIZE = 65536
|
||||
|
||||
|
||||
def _sanitize_header_filename(filename: Optional[str], fallback: str) -> str:
|
||||
"""Strip CRLF / quotes from a display filename for a Content-Disposition header."""
|
||||
if not filename:
|
||||
return fallback
|
||||
cleaned = re.sub(r'[\r\n"]', "", str(filename)).strip()
|
||||
return cleaned or fallback
|
||||
|
||||
|
||||
def _load_version(
|
||||
artifact_id: str, decoded_token: Optional[dict], query: Mapping[str, str]
|
||||
) -> Tuple[Optional[dict], Optional[Tuple[str, int]]]:
|
||||
"""Authorize the caller and fetch the requested version row; runs in a worker thread.
|
||||
|
||||
Returns:
|
||||
tuple: ``(version_row, None)`` or ``(None, (message, status))``.
|
||||
"""
|
||||
principal = principal_for(decoded_token, query.get("api_key"))
|
||||
with db_readonly() as conn:
|
||||
repo = ArtifactsRepository(conn)
|
||||
artifact = repo.get_artifact(artifact_id)
|
||||
if artifact is None:
|
||||
return None, ("Artifact not found", 404)
|
||||
if not authorize_artifact(conn, artifact, principal, args=query):
|
||||
return None, ("Forbidden", 403)
|
||||
version = artifact.get("current_version")
|
||||
version_arg = query.get("version")
|
||||
if version_arg is not None:
|
||||
try:
|
||||
version = int(version_arg)
|
||||
except ValueError:
|
||||
return None, ("Invalid version", 400)
|
||||
version_row = repo.get_version(artifact_id, version)
|
||||
if version_row is None:
|
||||
return None, ("Version not found", 404)
|
||||
return version_row, None
|
||||
|
||||
|
||||
async def _read_chunks(file_obj: BinaryIO) -> AsyncIterator[bytes]:
|
||||
"""Yield the file in chunks; reads from disk or a socket hop to a worker thread."""
|
||||
in_memory = isinstance(file_obj, io.BytesIO)
|
||||
while True:
|
||||
if in_memory:
|
||||
chunk = file_obj.read(_CHUNK_SIZE)
|
||||
else:
|
||||
chunk = await anyio.to_thread.run_sync(file_obj.read, _CHUNK_SIZE)
|
||||
if not chunk:
|
||||
return
|
||||
yield chunk
|
||||
|
||||
|
||||
async def _close_file(file_obj: BinaryIO) -> None:
|
||||
close = getattr(file_obj, "close", None)
|
||||
if callable(close):
|
||||
close()
|
||||
|
||||
|
||||
async def download_artifact(request: Request) -> Response:
|
||||
"""Serve an artifact version: a 302 (or a JSON URL envelope) on S3, streamed bytes otherwise."""
|
||||
artifact_id = request.path_params["artifact_id"]
|
||||
if not looks_like_uuid(artifact_id):
|
||||
return json_error("Artifact not found", 404)
|
||||
decoded, error = await authenticate(request)
|
||||
if error is not None:
|
||||
return error
|
||||
bind_log_context("artifact_download", decoded.get("sub") if decoded else None)
|
||||
query = request.query_params
|
||||
|
||||
try:
|
||||
version_row, failure = await anyio.to_thread.run_sync(
|
||||
_load_version, artifact_id, decoded, query
|
||||
)
|
||||
if failure is not None:
|
||||
return json_error(*failure)
|
||||
|
||||
# The object key is derived only from the stored path, never client input.
|
||||
storage_path = version_row.get("storage_path")
|
||||
if not storage_path:
|
||||
return json_error("No file for this version", 404)
|
||||
|
||||
filename = _sanitize_header_filename(version_row.get("filename"), f"artifact-{artifact_id}")
|
||||
mime_type = version_row.get("mime_type") or "application/octet-stream"
|
||||
storage = await anyio.to_thread.run_sync(StorageCreator.get_storage)
|
||||
|
||||
# With URL_STRATEGY=="s3" the contract is to hand back a presigned
|
||||
# URL. If the active backend can't mint one, that's a config error:
|
||||
# surface a 500 rather than silently proxying bytes from a backend
|
||||
# the operator expected to be off the hot path.
|
||||
if getattr(settings, "URL_STRATEGY", "backend") == "s3":
|
||||
try:
|
||||
url = await anyio.to_thread.run_sync(
|
||||
partial(storage.generate_presigned_url, storage_path, expires_in=_PRESIGNED_URL_TTL)
|
||||
)
|
||||
except NotImplementedError:
|
||||
logger.error(
|
||||
"URL_STRATEGY=s3 but %s cannot mint presigned URLs",
|
||||
type(storage).__name__,
|
||||
)
|
||||
return json_error("Storage misconfigured", 500)
|
||||
# A 302 to a cross-origin S3 URL can't be read by the app's authed
|
||||
# fetch (the bucket has no CORS grant for the app origin). When the
|
||||
# client opts in via ?disposition=url (or Accept: application/json)
|
||||
# hand the presigned URL back as JSON so it can navigate to it
|
||||
# top-level (no CORS). Default stays a 302 so nothing else breaks.
|
||||
if query.get("disposition") == "url" or (
|
||||
"application/json" in request.headers.get("Accept", "")
|
||||
):
|
||||
# Tag the envelope with a distinctive media type so the client
|
||||
# keys off a server-set signal, not the JSON body shape. A
|
||||
# ``data`` artifact whose bytes happen to be
|
||||
# ``{"success":true,"url":"..."}`` is streamed under the backend
|
||||
# strategy with its own content-type and can never carry this
|
||||
# vendor type, so it can't be mistaken for a redirect
|
||||
# (open-redirect gadget). Content-Type is CORS-safelisted.
|
||||
return JSONResponse(
|
||||
{"success": True, "url": url}, media_type=_ARTIFACT_URL_ENVELOPE_MIME
|
||||
)
|
||||
return RedirectResponse(url, status_code=302)
|
||||
|
||||
# Stream the bytes in chunks instead of buffering the whole object in
|
||||
# memory (artifacts can be many MB); the handle closes when the
|
||||
# response ends, including on a client disconnect.
|
||||
file_obj = await anyio.to_thread.run_sync(storage.get_file, storage_path)
|
||||
return ClosingStreamingResponse(
|
||||
_read_chunks(file_obj),
|
||||
media_type=mime_type,
|
||||
headers={"Content-Disposition": f'attachment; filename="{filename}"'},
|
||||
on_close=partial(_close_file, file_obj),
|
||||
)
|
||||
except FileNotFoundError:
|
||||
return json_error("File not found", 404)
|
||||
except Exception:
|
||||
logger.error("Error downloading artifact %s", artifact_id, exc_info=True)
|
||||
return JSONResponse({"success": False}, status_code=400)
|
||||
|
||||
|
||||
# Mounted in ``docsgpt/asgi.py`` ahead of the Flask catch-all. The other
|
||||
# artifact routes stay on Flask (``routes.py``).
|
||||
artifact_download_routes = [
|
||||
Route("/api/artifacts/{artifact_id}/download", download_artifact, methods=["GET"]),
|
||||
]
|
||||
@@ -1,18 +1,15 @@
|
||||
"""Artifact metadata and download routes (parent-derived authz)."""
|
||||
"""Artifact metadata, restore and delete routes (parent-derived authz).
|
||||
|
||||
Downloads are served by the native-async route in ``download.py``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from typing import Optional
|
||||
|
||||
from flask import (
|
||||
Response,
|
||||
current_app,
|
||||
jsonify,
|
||||
make_response,
|
||||
redirect,
|
||||
request,
|
||||
stream_with_context,
|
||||
)
|
||||
from flask_restx import Namespace, Resource
|
||||
|
||||
@@ -24,7 +21,6 @@ from docsgpt.api.user.artifacts.authz import (
|
||||
resolve_principal,
|
||||
user_can_access_conversation,
|
||||
)
|
||||
from docsgpt.core.settings import settings
|
||||
from docsgpt.storage.db.base_repository import looks_like_uuid
|
||||
from docsgpt.storage.db.repositories.artifacts import ArtifactsRepository
|
||||
from docsgpt.storage.db.repositories.conversations import ConversationsRepository
|
||||
@@ -34,20 +30,6 @@ from docsgpt.storage.storage_creator import StorageCreator
|
||||
|
||||
artifacts_ns = Namespace("artifacts", description="Artifact operations", path="/api")
|
||||
|
||||
# Presigned-URL TTL for private S3 artifact downloads (seconds).
|
||||
_PRESIGNED_URL_TTL = 300
|
||||
|
||||
|
||||
_ARTIFACT_URL_ENVELOPE_MIME = "application/vnd.docsgpt.artifact-url+json"
|
||||
|
||||
|
||||
def _sanitize_header_filename(filename: Optional[str], fallback: str) -> str:
|
||||
"""Strip CRLF / quotes from a display filename for a Content-Disposition header."""
|
||||
if not filename:
|
||||
return fallback
|
||||
cleaned = re.sub(r'[\r\n"]', "", str(filename)).strip()
|
||||
return cleaned or fallback
|
||||
|
||||
|
||||
def _artifact_summary(artifact: dict) -> dict:
|
||||
"""Project an artifact identity row to its API metadata shape (owner id withheld)."""
|
||||
@@ -255,103 +237,6 @@ class GetArtifactVersion(Resource):
|
||||
return make_response(jsonify({"success": False}), 400)
|
||||
|
||||
|
||||
@artifacts_ns.route("/artifacts/<artifact_id>/download")
|
||||
class DownloadArtifact(Resource):
|
||||
@api.doc(description="Download an artifact's bytes (302 to a presigned URL on S3)")
|
||||
def get(self, artifact_id: str):
|
||||
if not looks_like_uuid(artifact_id):
|
||||
return make_response(jsonify({"success": False, "message": "Artifact not found"}), 404)
|
||||
principal = resolve_principal()
|
||||
version_arg = request.args.get("version")
|
||||
try:
|
||||
with db_readonly() as conn:
|
||||
repo = ArtifactsRepository(conn)
|
||||
artifact = repo.get_artifact(artifact_id)
|
||||
if artifact is None:
|
||||
return make_response(jsonify({"success": False, "message": "Artifact not found"}), 404)
|
||||
if not authorize_artifact(conn, artifact, principal):
|
||||
return make_response(jsonify({"success": False, "message": "Forbidden"}), 403)
|
||||
version = artifact.get("current_version")
|
||||
if version_arg is not None:
|
||||
try:
|
||||
version = int(version_arg)
|
||||
except ValueError:
|
||||
return make_response(jsonify({"success": False, "message": "Invalid version"}), 400)
|
||||
version_row = repo.get_version(artifact_id, version)
|
||||
|
||||
if version_row is None:
|
||||
return make_response(jsonify({"success": False, "message": "Version not found"}), 404)
|
||||
# The object key is derived only from the stored path, never client input.
|
||||
storage_path = version_row.get("storage_path")
|
||||
if not storage_path:
|
||||
return make_response(jsonify({"success": False, "message": "No file for this version"}), 404)
|
||||
|
||||
filename = _sanitize_header_filename(version_row.get("filename"), f"artifact-{artifact_id}")
|
||||
mime_type = version_row.get("mime_type") or "application/octet-stream"
|
||||
storage = StorageCreator.get_storage()
|
||||
|
||||
# With URL_STRATEGY=="s3" the contract is to hand back a presigned
|
||||
# URL. If the active backend can't mint one, that's a config error:
|
||||
# surface a 500 rather than silently proxying bytes from a backend
|
||||
# the operator expected to be off the hot path.
|
||||
if getattr(settings, "URL_STRATEGY", "backend") == "s3":
|
||||
try:
|
||||
url = storage.generate_presigned_url(storage_path, expires_in=_PRESIGNED_URL_TTL)
|
||||
except NotImplementedError:
|
||||
current_app.logger.error(
|
||||
"URL_STRATEGY=s3 but %s cannot mint presigned URLs",
|
||||
type(storage).__name__,
|
||||
)
|
||||
return make_response(
|
||||
jsonify({"success": False, "message": "Storage misconfigured"}),
|
||||
500,
|
||||
)
|
||||
# A 302 to a cross-origin S3 URL can't be read by the app's authed
|
||||
# fetch (the bucket has no CORS grant for the app origin). When the
|
||||
# client opts in via ?disposition=url (or Accept: application/json)
|
||||
# hand the presigned URL back as JSON so it can navigate to it
|
||||
# top-level (no CORS). Default stays a 302 so nothing else breaks.
|
||||
if request.args.get("disposition") == "url" or (
|
||||
"application/json" in request.headers.get("Accept", "")
|
||||
):
|
||||
resp = make_response(jsonify({"success": True, "url": url}), 200)
|
||||
# Tag the envelope with a distinctive media type so the client
|
||||
# keys off a server-set signal, not the JSON body shape. A
|
||||
# ``data`` artifact whose bytes happen to be
|
||||
# ``{"success":true,"url":"..."}`` is streamed under the backend
|
||||
# strategy with its own (``application/json``) content-type and
|
||||
# can never carry this vendor type, so it can't be mistaken for a
|
||||
# redirect (open-redirect gadget). Content-Type is CORS-safelisted,
|
||||
# so this stays readable cross-origin without expose-headers.
|
||||
resp.headers["Content-Type"] = _ARTIFACT_URL_ENVELOPE_MIME
|
||||
return resp
|
||||
return redirect(url, code=302)
|
||||
|
||||
# Stream the bytes in chunks instead of buffering the whole object in
|
||||
# worker memory (artifacts can be many MB); close the handle when done.
|
||||
file_obj = storage.get_file(storage_path)
|
||||
|
||||
def _stream():
|
||||
try:
|
||||
for chunk in iter(lambda: file_obj.read(65536), b""):
|
||||
yield chunk
|
||||
finally:
|
||||
close = getattr(file_obj, "close", None)
|
||||
if callable(close):
|
||||
close()
|
||||
|
||||
return Response(
|
||||
stream_with_context(_stream()),
|
||||
mimetype=mime_type,
|
||||
headers={"Content-Disposition": f'attachment; filename="{filename}"'},
|
||||
)
|
||||
except FileNotFoundError:
|
||||
return make_response(jsonify({"success": False, "message": "File not found"}), 404)
|
||||
except Exception as err:
|
||||
current_app.logger.error(f"Error downloading artifact: {err}", exc_info=True)
|
||||
return make_response(jsonify({"success": False}), 400)
|
||||
|
||||
|
||||
@artifacts_ns.route("/artifacts/<artifact_id>/restore")
|
||||
class RestoreArtifact(Resource):
|
||||
@api.doc(description="Restore a prior version by appending it as the new current version")
|
||||
|
||||
Reference in new issue
Block a user