Merge pull request #600 from arabcoders/dev
fix: Handle TimeoutError in fetch_info to prevent double wait
This commit is contained in:
commit
522e6a75ef
15 changed files with 348 additions and 14 deletions
|
|
@ -1,3 +1,4 @@
|
|||
import asyncio
|
||||
import logging
|
||||
from collections import OrderedDict
|
||||
from typing import Any
|
||||
|
|
@ -85,7 +86,7 @@ async def conditions_test(request: Request, encoder: Encoder, cache: Cache, conf
|
|||
return web.json_response({"error": "condition is required."}, status=web.HTTPBadRequest.status_code)
|
||||
|
||||
try:
|
||||
validate_url(url, allow_internal=config.allow_internal_urls)
|
||||
await asyncio.to_thread(validate_url, url, config.allow_internal_urls)
|
||||
except ValueError as e:
|
||||
return web.json_response({"error": str(e)}, status=web.HTTPBadRequest.status_code)
|
||||
|
||||
|
|
|
|||
|
|
@ -187,6 +187,7 @@ class GenericTaskHandler(BaseHandler):
|
|||
url=url,
|
||||
no_archive=True,
|
||||
no_log=True,
|
||||
budget_sleep=True,
|
||||
)
|
||||
|
||||
if not info:
|
||||
|
|
|
|||
|
|
@ -185,6 +185,7 @@ class RssGenericHandler(BaseHandler):
|
|||
url=url,
|
||||
no_archive=True,
|
||||
no_log=True,
|
||||
budget_sleep=True,
|
||||
)
|
||||
|
||||
if not info:
|
||||
|
|
|
|||
|
|
@ -107,6 +107,7 @@ class HandleTask(TaskSchema):
|
|||
no_archive=True,
|
||||
follow_redirect=False,
|
||||
sanitize_info=True,
|
||||
budget_sleep=True,
|
||||
)
|
||||
|
||||
if not ie_info or not isinstance(ie_info, dict):
|
||||
|
|
@ -133,7 +134,7 @@ class HandleTask(TaskSchema):
|
|||
|
||||
archive_file: Path = Path(archive_file)
|
||||
|
||||
(ie_info, _) = await fetch_info(params, self.url, no_archive=True, follow_redirect=True)
|
||||
(ie_info, _) = await fetch_info(params, self.url, no_archive=True, follow_redirect=True, budget_sleep=True)
|
||||
if not ie_info or not isinstance(ie_info, dict):
|
||||
return (False, "Failed to extract information from URL.")
|
||||
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import asyncio
|
||||
import logging
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
|
|
@ -469,7 +470,7 @@ async def task_handler_inspect(request: Request, handler: TaskHandle, encoder: E
|
|||
static_only: bool = data.get("static_only", False) if isinstance(data, dict) else False
|
||||
if not static_only:
|
||||
try:
|
||||
validate_url(url, allow_internal=config.allow_internal_urls)
|
||||
await asyncio.to_thread(validate_url, url, config.allow_internal_urls)
|
||||
except ValueError as e:
|
||||
return web.json_response({"error": str(e)}, status=web.HTTPBadRequest.status_code)
|
||||
|
||||
|
|
@ -710,7 +711,7 @@ async def task_metadata(request: Request, repo: TasksRepository, config: Config,
|
|||
continue
|
||||
|
||||
try:
|
||||
validate_url(url, allow_internal=config.allow_internal_urls)
|
||||
await asyncio.to_thread(validate_url, url, config.allow_internal_urls)
|
||||
except ValueError:
|
||||
LOG.warning(f"Invalid thumbnail url '{url}'")
|
||||
continue
|
||||
|
|
|
|||
|
|
@ -49,6 +49,17 @@ class ExtractorConfig:
|
|||
self.wait_threshold = wait_threshold
|
||||
|
||||
|
||||
def _sleep_timeout(config: dict[str, Any], timeout: float, budget_sleep: bool) -> float:
|
||||
if not budget_sleep:
|
||||
return timeout
|
||||
|
||||
sleep_requests = config.get("sleep_interval_requests")
|
||||
if not isinstance(sleep_requests, int | float) or sleep_requests <= 0:
|
||||
return timeout
|
||||
|
||||
return timeout + min(float(sleep_requests) * 20, 300.0)
|
||||
|
||||
|
||||
class ExtractorPool(metaclass=Singleton):
|
||||
"""
|
||||
Manages process pool and semaphore for video information extraction.
|
||||
|
|
@ -312,6 +323,7 @@ async def fetch_info(
|
|||
sanitize_info: bool = False,
|
||||
capture_logs: int | None = None,
|
||||
extractor_config: ExtractorConfig | None = None,
|
||||
budget_sleep: bool = False,
|
||||
**kwargs,
|
||||
) -> tuple[dict[str, Any] | None, list[dict[str, Any]]]:
|
||||
"""
|
||||
|
|
@ -329,6 +341,7 @@ async def fetch_info(
|
|||
sanitize_info: Sanitize the extracted information
|
||||
capture_logs: If provided (e.g., logging.WARNING), capture logs
|
||||
extractor_config: Configuration for the extractor
|
||||
budget_sleep: Whether to add extra timeout budget for request-sleep-heavy extraction
|
||||
**kwargs: Additional arguments
|
||||
|
||||
Returns:
|
||||
|
|
@ -352,6 +365,7 @@ async def fetch_info(
|
|||
loop = asyncio.get_running_loop()
|
||||
|
||||
safe_config = _sanitize_config(config)
|
||||
timeout = _sleep_timeout(safe_config, extractor_config.timeout, budget_sleep)
|
||||
|
||||
try:
|
||||
try:
|
||||
|
|
@ -372,9 +386,12 @@ async def fetch_info(
|
|||
**kwargs,
|
||||
),
|
||||
),
|
||||
timeout=extractor_config.timeout,
|
||||
timeout=timeout,
|
||||
)
|
||||
|
||||
except TimeoutError:
|
||||
raise
|
||||
|
||||
except Exception as exc:
|
||||
LOG.exception(exc)
|
||||
LOG.warning("extract_info process pool failed, falling back to thread pool url=%s error=%s", url, exc)
|
||||
|
|
@ -393,7 +410,7 @@ async def fetch_info(
|
|||
**kwargs,
|
||||
),
|
||||
),
|
||||
timeout=extractor_config.timeout,
|
||||
timeout=timeout,
|
||||
)
|
||||
finally:
|
||||
semaphore.release()
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import asyncio
|
||||
import json
|
||||
import logging
|
||||
import time
|
||||
|
|
@ -301,7 +302,7 @@ async def get_info(request: Request, cache: Cache, config: Config) -> Response:
|
|||
)
|
||||
|
||||
try:
|
||||
validate_url(url, allow_internal=config.allow_internal_urls)
|
||||
await asyncio.to_thread(validate_url, url, config.allow_internal_urls)
|
||||
except ValueError as e:
|
||||
return web.json_response(
|
||||
data={"status": False, "message": str(e), "error": str(e)},
|
||||
|
|
@ -453,7 +454,7 @@ async def get_archive_ids(request: Request, config: Config) -> Response:
|
|||
for i, url in enumerate(data):
|
||||
dct = {"index": i, "url": url}
|
||||
try:
|
||||
validate_url(url, allow_internal=config.allow_internal_urls)
|
||||
await asyncio.to_thread(validate_url, url, config.allow_internal_urls)
|
||||
dct.update(get_archive_id(url))
|
||||
except ValueError as e:
|
||||
dct.update({"id": None, "ie_key": None, "archive_id": None, "error": str(e)})
|
||||
|
|
|
|||
|
|
@ -208,6 +208,7 @@ async def add(
|
|||
no_archive=False,
|
||||
follow_redirect=True,
|
||||
capture_logs=logging.WARNING,
|
||||
budget_sleep=True,
|
||||
)
|
||||
|
||||
if not entry:
|
||||
|
|
|
|||
|
|
@ -792,6 +792,7 @@ async def item_nfo_generate(request: Request, queue: DownloadQueue) -> Response:
|
|||
url=item.info.url,
|
||||
no_archive=True,
|
||||
follow_redirect=True,
|
||||
budget_sleep=True,
|
||||
)
|
||||
|
||||
if not info_dict:
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import asyncio
|
||||
import logging
|
||||
import random
|
||||
import time
|
||||
|
|
@ -39,7 +40,7 @@ async def get_thumbnail(request: Request, config: Config) -> Response:
|
|||
return web.json_response(data={"error": "URL is required."}, status=web.HTTPForbidden.status_code)
|
||||
|
||||
try:
|
||||
validate_url(url, allow_internal=config.allow_internal_urls)
|
||||
await asyncio.to_thread(validate_url, url, config.allow_internal_urls)
|
||||
except ValueError as e:
|
||||
return web.json_response(data={"error": str(e)}, status=web.HTTPForbidden.status_code)
|
||||
|
||||
|
|
@ -59,6 +60,7 @@ async def get_thumbnail(request: Request, config: Config) -> Response:
|
|||
url=url,
|
||||
follow_redirects=True,
|
||||
headers=request_headers,
|
||||
timeout=10.0,
|
||||
)
|
||||
|
||||
if response.status_code != web.HTTPOk.status_code:
|
||||
|
|
|
|||
120
app/tests/test_async_url_validation_routes.py
Normal file
120
app/tests/test_async_url_validation_routes.py
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from typing import Any, Generator
|
||||
|
||||
import pytest
|
||||
|
||||
from app.features.conditions import router as conditions_router
|
||||
from app.features.tasks import router as tasks_router
|
||||
from app.features.ytdlp import router as ytdlp_router
|
||||
from app.library.config import Config
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def reset_config() -> Generator[None, None, None]:
|
||||
Config._reset_singleton()
|
||||
yield
|
||||
Config._reset_singleton()
|
||||
|
||||
|
||||
class _Req:
|
||||
def __init__(self, payload: Any) -> None:
|
||||
self._payload = payload
|
||||
self.body_exists = payload is not None
|
||||
|
||||
async def json(self) -> Any:
|
||||
return self._payload
|
||||
|
||||
|
||||
class _InspectReq(_Req):
|
||||
query: dict[str, str] = {}
|
||||
match_info: dict[str, str] = {}
|
||||
|
||||
|
||||
class _QueryReq:
|
||||
def __init__(self, query: dict[str, str]) -> None:
|
||||
self.query = query
|
||||
|
||||
|
||||
def _patch_thread(monkeypatch: pytest.MonkeyPatch, module: Any, config: Config, url: str) -> dict[str, bool]:
|
||||
seen = {"to_thread": False, "validate": False}
|
||||
|
||||
def fake_validate_url(next_url: str, allow_internal: bool = False) -> bool:
|
||||
seen["validate"] = True
|
||||
assert next_url == url
|
||||
assert allow_internal is config.allow_internal_urls
|
||||
raise ValueError("Invalid hostname.")
|
||||
|
||||
async def fake_to_thread(func, *args, **kwargs):
|
||||
seen["to_thread"] = True
|
||||
return func(*args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(module, "validate_url", fake_validate_url)
|
||||
monkeypatch.setattr(module.asyncio, "to_thread", fake_to_thread)
|
||||
return seen
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_inspect_thread(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
config = Config.get_instance()
|
||||
request = _InspectReq({"url": "https://bad.example/task"})
|
||||
seen = _patch_thread(monkeypatch, tasks_router, config, "https://bad.example/task")
|
||||
|
||||
response = await tasks_router.task_handler_inspect(request, handler=None, encoder=None, config=config)
|
||||
|
||||
assert response.status == 400
|
||||
assert json.loads(response.body.decode("utf-8")) == {"error": "Invalid hostname."}
|
||||
assert seen == {"to_thread": True, "validate": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_conditions_thread(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
config = Config.get_instance()
|
||||
request = _Req({"url": "https://bad.example/cond", "condition": "title ~= 'x'"})
|
||||
seen = _patch_thread(monkeypatch, conditions_router, config, "https://bad.example/cond")
|
||||
|
||||
response = await conditions_router.conditions_test(request, encoder=None, cache=None, config=config)
|
||||
|
||||
assert response.status == 400
|
||||
assert json.loads(response.body.decode("utf-8")) == {"error": "Invalid hostname."}
|
||||
assert seen == {"to_thread": True, "validate": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_info_thread(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
config = Config.get_instance()
|
||||
request = _QueryReq({"url": "https://bad.example/info"})
|
||||
seen = _patch_thread(monkeypatch, ytdlp_router, config, "https://bad.example/info")
|
||||
|
||||
response = await ytdlp_router.get_info(request, cache=None, config=config)
|
||||
|
||||
assert response.status == 400
|
||||
assert json.loads(response.body.decode("utf-8")) == {
|
||||
"status": False,
|
||||
"message": "Invalid hostname.",
|
||||
"error": "Invalid hostname.",
|
||||
}
|
||||
assert seen == {"to_thread": True, "validate": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_archive_ids_thread(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
config = Config.get_instance()
|
||||
request = _Req(["https://bad.example/archive"])
|
||||
seen = _patch_thread(monkeypatch, ytdlp_router, config, "https://bad.example/archive")
|
||||
|
||||
response = await ytdlp_router.get_archive_ids(request, config)
|
||||
|
||||
assert response.status == 200
|
||||
assert json.loads(response.body.decode("utf-8")) == [
|
||||
{
|
||||
"index": 0,
|
||||
"url": "https://bad.example/archive",
|
||||
"id": None,
|
||||
"ie_key": None,
|
||||
"archive_id": None,
|
||||
"error": "Invalid hostname.",
|
||||
}
|
||||
]
|
||||
assert seen == {"to_thread": True, "validate": True}
|
||||
94
app/tests/test_extractor_timeout.py
Normal file
94
app/tests/test_extractor_timeout.py
Normal file
|
|
@ -0,0 +1,94 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from unittest.mock import Mock
|
||||
|
||||
import pytest
|
||||
|
||||
from app.features.ytdlp import extractor
|
||||
|
||||
|
||||
class _Loop:
|
||||
def __init__(self) -> None:
|
||||
self.calls: list[object | None] = []
|
||||
|
||||
def run_in_executor(self, executor, func): # noqa: ANN001
|
||||
self.calls.append(executor)
|
||||
return func
|
||||
|
||||
|
||||
class _Pool:
|
||||
def __init__(self) -> None:
|
||||
self.semaphore = asyncio.Semaphore(1)
|
||||
self.executor = object()
|
||||
|
||||
def get_semaphore(self, _config: extractor.ExtractorConfig) -> asyncio.Semaphore:
|
||||
return self.semaphore
|
||||
|
||||
def get_pool(self, _config: extractor.ExtractorConfig) -> object:
|
||||
return self.executor
|
||||
|
||||
|
||||
def test_sleep_budget() -> None:
|
||||
assert extractor._sleep_timeout({}, 70, False) == 70
|
||||
assert extractor._sleep_timeout({"sleep_interval_requests": 0}, 70, True) == 70
|
||||
assert extractor._sleep_timeout({"sleep_interval_requests": 3}, 70, True) == 130
|
||||
assert extractor._sleep_timeout({"sleep_interval_requests": 30}, 70, True) == 370
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_timeout_no_retry(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
pool = _Pool()
|
||||
loop = _Loop()
|
||||
seen: list[float] = []
|
||||
|
||||
async def fake_wait_for(*, fut, timeout):
|
||||
seen.append(timeout)
|
||||
raise TimeoutError
|
||||
|
||||
monkeypatch.setattr(extractor.ExtractorPool, "get_instance", classmethod(lambda cls: pool))
|
||||
monkeypatch.setattr(extractor.asyncio, "get_running_loop", lambda: loop)
|
||||
monkeypatch.setattr(extractor.asyncio, "wait_for", fake_wait_for)
|
||||
|
||||
with pytest.raises(TimeoutError):
|
||||
await extractor.fetch_info(
|
||||
config={"sleep_interval_requests": 3},
|
||||
url="https://example.com",
|
||||
extractor_config=extractor.ExtractorConfig(concurrency=1, timeout=70),
|
||||
budget_sleep=True,
|
||||
)
|
||||
|
||||
assert loop.calls == [pool.executor]
|
||||
assert seen == [130]
|
||||
assert not pool.semaphore.locked()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pool_fallback(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
pool = _Pool()
|
||||
loop = _Loop()
|
||||
expected = ({"id": "ok"}, [])
|
||||
seen: list[float] = []
|
||||
|
||||
async def fake_wait_for(*, fut, timeout):
|
||||
seen.append(timeout)
|
||||
if len(loop.calls) == 1:
|
||||
raise RuntimeError("pool failed")
|
||||
return fut()
|
||||
|
||||
monkeypatch.setattr(extractor.ExtractorPool, "get_instance", classmethod(lambda cls: pool))
|
||||
monkeypatch.setattr(extractor.asyncio, "get_running_loop", lambda: loop)
|
||||
monkeypatch.setattr(extractor.asyncio, "wait_for", fake_wait_for)
|
||||
monkeypatch.setattr(extractor, "extract_info_sync", Mock(return_value=expected))
|
||||
|
||||
result = await extractor.fetch_info(
|
||||
config={"sleep_interval_requests": 3},
|
||||
url="https://example.com",
|
||||
extractor_config=extractor.ExtractorConfig(concurrency=1, timeout=70),
|
||||
budget_sleep=True,
|
||||
)
|
||||
|
||||
assert result == expected
|
||||
assert loop.calls == [pool.executor, None]
|
||||
assert seen == [130, 130]
|
||||
assert not pool.semaphore.locked()
|
||||
97
app/tests/test_images_routes.py
Normal file
97
app/tests/test_images_routes.py
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from typing import Generator
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
from aiohttp import web
|
||||
from aiohttp.test_utils import make_mocked_request
|
||||
|
||||
from app.library.config import Config
|
||||
from app.routes.api import images
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def reset_config() -> Generator[None, None, None]:
|
||||
Config._reset_singleton()
|
||||
yield
|
||||
Config._reset_singleton()
|
||||
|
||||
|
||||
class _Resp:
|
||||
def __init__(self, *, status_code: int = 200, content: bytes = b"img", content_type: str = "image/jpeg") -> None:
|
||||
self.status_code = status_code
|
||||
self.content = content
|
||||
self.headers = {"Content-Type": content_type}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_thumb_thread(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
config = Config.get_instance()
|
||||
req = make_mocked_request("GET", "/api/thumbnail?url=https://example.com/a.jpg")
|
||||
req._rel_url = req._rel_url.with_query({"url": "https://example.com/a.jpg"})
|
||||
|
||||
seen = {"to_thread": False, "validate": False}
|
||||
|
||||
def fake_validate_url(url: str, allow_internal: bool = False) -> bool:
|
||||
seen["validate"] = True
|
||||
assert url == "https://example.com/a.jpg"
|
||||
assert allow_internal is config.allow_internal_urls
|
||||
return True
|
||||
|
||||
async def fake_to_thread(func, *args, **kwargs):
|
||||
seen["to_thread"] = True
|
||||
return func(*args, **kwargs)
|
||||
|
||||
client = AsyncMock()
|
||||
client.request.return_value = _Resp()
|
||||
|
||||
monkeypatch.setattr(images, "validate_url", fake_validate_url)
|
||||
monkeypatch.setattr(images.asyncio, "to_thread", fake_to_thread)
|
||||
monkeypatch.setattr(images, "get_async_client", lambda **_kwargs: client)
|
||||
monkeypatch.setattr(images, "resolve_curl_transport", lambda: False)
|
||||
monkeypatch.setattr(images, "build_request_headers", lambda **_kwargs: {})
|
||||
monkeypatch.setattr(images.Globals, "get_random_agent", staticmethod(lambda: "agent"))
|
||||
monkeypatch.setattr(
|
||||
images.YTDLPOpts,
|
||||
"get_instance",
|
||||
staticmethod(
|
||||
lambda: type(
|
||||
"Opts",
|
||||
(),
|
||||
{
|
||||
"preset": lambda self, name: self,
|
||||
"get_all": lambda self: {},
|
||||
},
|
||||
)()
|
||||
),
|
||||
)
|
||||
|
||||
response = await images.get_thumbnail(req, config)
|
||||
|
||||
assert response.status == web.HTTPOk.status_code
|
||||
assert seen["to_thread"] is True
|
||||
assert seen["validate"] is True
|
||||
client.request.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_thumb_reject(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
config = Config.get_instance()
|
||||
req = make_mocked_request("GET", "/api/thumbnail?url=https://bad.example/a.jpg")
|
||||
req._rel_url = req._rel_url.with_query({"url": "https://bad.example/a.jpg"})
|
||||
|
||||
def fake_validate_url(_url: str, allow_internal: bool = False) -> bool:
|
||||
assert allow_internal is config.allow_internal_urls
|
||||
raise ValueError("Invalid hostname.")
|
||||
|
||||
async def fake_to_thread(func, *args, **kwargs):
|
||||
return func(*args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(images, "validate_url", fake_validate_url)
|
||||
monkeypatch.setattr(images.asyncio, "to_thread", fake_to_thread)
|
||||
|
||||
response = await images.get_thumbnail(req, config)
|
||||
|
||||
assert response.status == web.HTTPForbidden.status_code
|
||||
assert response.text == '{"error": "Invalid hostname."}'
|
||||
|
|
@ -17,7 +17,7 @@
|
|||
</span>
|
||||
</template>
|
||||
|
||||
<div class="flex flex-col gap-2 sm:flex-row sm:items-start">
|
||||
<div class="flex flex-row gap-2 items-start">
|
||||
<div class="min-w-0 flex-1">
|
||||
<UTextarea
|
||||
v-if="isMultiLineInput"
|
||||
|
|
|
|||
|
|
@ -1119,10 +1119,6 @@ async function src_error(event: Event) {
|
|||
return;
|
||||
}
|
||||
|
||||
if (videoElement.value?.paused && !active.value) {
|
||||
return;
|
||||
}
|
||||
|
||||
console.warn('Source failed to load, attempting HLS fallback via hls.js...', event);
|
||||
attach_hls(currentPlaybackUrl('m3u8', true));
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue