mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(SKY-11992): capture blob: URL downloads in the CDP download monitor (#7192)
Co-authored-by: AronPerez <aperez0295@gmail.com>
This commit is contained in:
LawyZheng
GitHub
parent
b7c1383f81
commit
c78637ae87
4 changed files with 403 additions and 32 deletions
|
|
@ -32,6 +32,8 @@ from urllib.parse import unquote, urlparse
|
|||
import structlog
|
||||
from playwright.async_api import Browser, BrowserContext, CDPSession, Page
|
||||
|
||||
from skyvern.webeye.utils.page import SkyvernFrame
|
||||
|
||||
LOG = structlog.get_logger()
|
||||
|
||||
# Chunk size for IO.read streaming
|
||||
|
|
@ -455,22 +457,21 @@ class CDPDownloadInterceptor:
|
|||
LOG.warning("Empty download URL, skipping")
|
||||
return
|
||||
|
||||
# Skip if already downloaded via CDP Fetch interception.
|
||||
# Fetch always fires before Browser.downloadWillBegin (Fetch intercepts at
|
||||
# response stage, browser download manager fires after fulfillRequest), so
|
||||
# this check is purely one-directional: only _handle_download writes the set.
|
||||
# Skip if this exact URL was already captured. Both the Fetch path
|
||||
# (_handle_download) and the blob path (_download_blob_url, after a successful save)
|
||||
# record URLs here. We record only AFTER a successful save — not before the read — so
|
||||
# a transient failure can't block a later retry of the same URL; a rare duplicate
|
||||
# downloadWillBegin for the same blob URL is a benign re-save/overwrite.
|
||||
if url in self._downloaded_urls:
|
||||
LOG.debug("URL already captured via Fetch, skipping direct download", url=url)
|
||||
return
|
||||
|
||||
if url.startswith("blob:"):
|
||||
# blob: URLs are in-memory browser references — not fetchable over HTTP.
|
||||
# They are already handled by the Fetch path which intercepts the resolved blob.
|
||||
# TODO: handle the edge case where Fetch doesn't catch a blob download.
|
||||
LOG.warning(
|
||||
"blob: URL download not yet supported, skipping", url=url, suggested_filename=suggested_filename
|
||||
)
|
||||
return
|
||||
# blob: URLs are in-memory browser references — not fetchable over HTTP. When the
|
||||
# page builds the file client-side (e.g. Blob + createObjectURL), the CDP Fetch
|
||||
# path never sees a network response, so read the bytes back from a same-origin
|
||||
# page instead of dropping the download.
|
||||
await self._download_blob_url(url, suggested_filename)
|
||||
elif url.startswith("http"):
|
||||
await self._download_url_directly(url, suggested_filename)
|
||||
else:
|
||||
|
|
@ -556,6 +557,57 @@ class CDPDownloadInterceptor:
|
|||
method=method,
|
||||
)
|
||||
|
||||
async def _download_blob_url(self, url: str, suggested_filename: str) -> None:
|
||||
"""Save a blob: URL download by reading its bytes back from a same-origin page.
|
||||
|
||||
blob: URLs are in-memory references owned by the document that created them, so they
|
||||
can't be fetched over HTTP. ``SkyvernFrame.read_blob_url_bytes`` runs the shared blob
|
||||
read-back script inside a same-origin frame. Best-effort: a page may revoke the object
|
||||
URL before we read it.
|
||||
"""
|
||||
if not self._output_dir or self._browser_context is None:
|
||||
LOG.warning("Cannot read blob download: no output dir or browser context", url=url)
|
||||
return
|
||||
|
||||
# probe=True: this fans out over every open page as a best-effort fallback, so the
|
||||
# shared reader must not emit ERROR logs for pages that don't own the blob's origin.
|
||||
data: bytes | None = None
|
||||
for page in list(self._browser_context.pages):
|
||||
data = await SkyvernFrame.read_blob_url_bytes(
|
||||
page=page, blob_url=url, max_size_bytes=MAX_FILE_SIZE_BYTES, probe=True
|
||||
)
|
||||
if data is not None:
|
||||
break
|
||||
|
||||
if data is None:
|
||||
LOG.warning(
|
||||
"Could not read blob download from any page",
|
||||
url=url,
|
||||
suggested_filename=suggested_filename,
|
||||
)
|
||||
return
|
||||
# Defense-in-depth: read_blob_url_bytes already rejects oversized blobs in-page before
|
||||
# serialization, but guard again in case a caller passes no limit.
|
||||
if len(data) > MAX_FILE_SIZE_BYTES:
|
||||
LOG.warning(
|
||||
"Blob download exceeds size limit, discarding",
|
||||
url=url,
|
||||
size=len(data),
|
||||
max_size=MAX_FILE_SIZE_BYTES,
|
||||
)
|
||||
return
|
||||
save_path, filename = self._resolve_save_path(suggested_filename)
|
||||
with open(save_path, "wb") as f:
|
||||
f.write(data)
|
||||
self._downloaded_urls.add(url)
|
||||
LOG.info(
|
||||
"CDP download saved (blob)",
|
||||
filename=filename,
|
||||
size=len(data),
|
||||
save_path=str(save_path),
|
||||
download_index=self._download_index,
|
||||
)
|
||||
|
||||
async def disable(self) -> None:
|
||||
"""Disable Fetch interception on all CDP sessions and clean up browser monitor."""
|
||||
session_count = len(self._cdp_sessions)
|
||||
|
|
|
|||
|
|
@ -363,13 +363,19 @@ def _merge_images_by_position(images: list[Image.Image], positions: list[int]) -
|
|||
# FileReader keeps the payload binary-safe without arrayBuffer/Uint8Array
|
||||
# transcoding back across CDP.
|
||||
_BLOB_FETCH_JS = """
|
||||
async (blobUrl) => {
|
||||
async (args) => {
|
||||
try {
|
||||
const { blobUrl, maxSizeBytes } = args;
|
||||
const response = await fetch(blobUrl);
|
||||
if (!response.ok) {
|
||||
return { ok: false, status: response.status };
|
||||
}
|
||||
const blob = await response.blob();
|
||||
// Reject oversized blobs before serializing them to a data URL, so a huge
|
||||
// client-side blob can't be read fully into memory / base64-transcoded.
|
||||
if (maxSizeBytes != null && blob.size > maxSizeBytes) {
|
||||
return { ok: false, error: 'too_large', size: blob.size };
|
||||
}
|
||||
return await new Promise((resolve) => {
|
||||
const reader = new FileReader();
|
||||
reader.onloadend = () => {
|
||||
|
|
@ -430,6 +436,19 @@ def _frames_for_blob_origin(page: Page, blob_origin: str) -> list[Frame]:
|
|||
return matches
|
||||
|
||||
|
||||
def _all_page_frames(page: Page) -> list[Frame]:
|
||||
"""All frames on the page, main frame first, deduped."""
|
||||
seen: set[int] = set()
|
||||
frames: list[Frame] = []
|
||||
for frame in [page.main_frame, *page.frames]:
|
||||
frame_id = id(frame)
|
||||
if frame_id in seen:
|
||||
continue
|
||||
seen.add(frame_id)
|
||||
frames.append(frame)
|
||||
return frames
|
||||
|
||||
|
||||
class SkyvernFrame:
|
||||
@staticmethod
|
||||
async def evaluate(
|
||||
|
|
@ -565,23 +584,32 @@ class SkyvernFrame:
|
|||
page: Page,
|
||||
blob_url: str,
|
||||
workflow_run_id: str | None = None,
|
||||
max_size_bytes: int | None = None,
|
||||
probe: bool = False,
|
||||
) -> bytes | None:
|
||||
# probe=True is for best-effort multi-page fallback where the caller tries every open
|
||||
# page; expected misses on non-owning pages shouldn't spam ERROR/WARN logs, so downgrade
|
||||
# give-up/retry logging to debug. The final failure signal stays with the caller.
|
||||
give_up_log = LOG.debug if probe else LOG.error
|
||||
retry_log = LOG.debug if probe else LOG.warning
|
||||
|
||||
blob_origin = _blob_url_origin(blob_url)
|
||||
if blob_origin is None:
|
||||
LOG.error(
|
||||
"blob URL read aborted: unparseable origin",
|
||||
workflow_run_id=workflow_run_id,
|
||||
)
|
||||
if blob_origin is not None:
|
||||
frames = _frames_for_blob_origin(page, blob_origin)
|
||||
elif blob_url.startswith("blob:"):
|
||||
# Opaque-origin blobs (blob:null/...) from sandboxed iframes or data: documents have
|
||||
# no matchable origin — probe every frame since we can't identify the owner by origin.
|
||||
frames = _all_page_frames(page)
|
||||
else:
|
||||
give_up_log("blob URL read aborted: not a blob URL", workflow_run_id=workflow_run_id)
|
||||
return None
|
||||
|
||||
frames = _frames_for_blob_origin(page, blob_origin)
|
||||
if not frames:
|
||||
LOG.error(
|
||||
"blob URL read found no frame at the blob origin",
|
||||
workflow_run_id=workflow_run_id,
|
||||
)
|
||||
give_up_log("blob URL read found no candidate frame", workflow_run_id=workflow_run_id)
|
||||
return None
|
||||
|
||||
# blob.size is checked in-page against this before the payload is serialized.
|
||||
blob_arg = {"blobUrl": blob_url, "maxSizeBytes": max_size_bytes}
|
||||
main_frame = page.main_frame
|
||||
for frame in frames:
|
||||
try:
|
||||
|
|
@ -589,41 +617,49 @@ class SkyvernFrame:
|
|||
# context-level main-world prefix stays attached; sub-frames use
|
||||
# frame.evaluate (main-world prefixes are page-scoped).
|
||||
if frame is main_frame:
|
||||
result = await evaluate_in_main_world(page, _BLOB_FETCH_JS, blob_url)
|
||||
result = await evaluate_in_main_world(page, _BLOB_FETCH_JS, blob_arg)
|
||||
else:
|
||||
result = await frame.evaluate(_BLOB_FETCH_JS, blob_url)
|
||||
result = await frame.evaluate(_BLOB_FETCH_JS, blob_arg)
|
||||
except Exception:
|
||||
LOG.warning(
|
||||
retry_log(
|
||||
"blob URL in-frame fetch raised; trying next frame if any",
|
||||
workflow_run_id=workflow_run_id,
|
||||
exc_info=True,
|
||||
)
|
||||
continue
|
||||
if not isinstance(result, dict) or not result.get("ok"):
|
||||
if isinstance(result, dict) and result.get("error") == "too_large":
|
||||
LOG.warning(
|
||||
"blob URL exceeds max size; not reading",
|
||||
workflow_run_id=workflow_run_id,
|
||||
size=result.get("size"),
|
||||
max_size_bytes=max_size_bytes,
|
||||
)
|
||||
return None
|
||||
if not isinstance(result, dict) or not result.get("ok"):
|
||||
retry_log(
|
||||
"blob URL in-frame fetch returned not-ok; trying next frame if any",
|
||||
workflow_run_id=workflow_run_id,
|
||||
result=result if isinstance(result, dict) else None,
|
||||
)
|
||||
continue
|
||||
b64_payload = result.get("base64")
|
||||
if not isinstance(b64_payload, str) or not b64_payload:
|
||||
LOG.warning(
|
||||
"blob URL in-frame fetch returned empty payload; trying next frame if any",
|
||||
if not isinstance(b64_payload, str):
|
||||
retry_log(
|
||||
"blob URL in-frame fetch returned non-string payload; trying next frame if any",
|
||||
workflow_run_id=workflow_run_id,
|
||||
)
|
||||
continue
|
||||
try:
|
||||
return base64.b64decode(b64_payload, validate=True)
|
||||
except Exception:
|
||||
LOG.warning(
|
||||
retry_log(
|
||||
"blob URL in-frame fetch payload was not valid base64; trying next frame if any",
|
||||
workflow_run_id=workflow_run_id,
|
||||
exc_info=True,
|
||||
)
|
||||
continue
|
||||
|
||||
LOG.error(
|
||||
give_up_log(
|
||||
"blob URL read could not retrieve bytes from any matching frame",
|
||||
workflow_run_id=workflow_run_id,
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue