Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions music_assistant/helpers/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2204,17 +2204,17 @@ async def wrapper(self: SelfT, *args: P.args, **kwargs: P.kwargs) -> R:
),
)
)
# the coroutine is built here rather than passing func and its arguments on: a
# wrapped function is free to name a parameter after one of the task options below,
# which forwarded kwargs would collide with
task: asyncio.Task[R] = mass.create_task(
func,
self,
*args,
func(self, *args, **kwargs),
task_id=task_id,
abort_existing=False,
eager_start=True,
# every caller awaits the flight below and so sees the failure itself; the
# task's own exception log would report a handled error as an unhandled one
log_exceptions=False,
**kwargs,
)
return await join_task(task)

Expand Down
23 changes: 18 additions & 5 deletions music_assistant/mass.py
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ def create_task(
target: Callable[..., Coroutine[Any, Any, _R]] | Awaitable[_R],
*args: Any,
task_id: str | None = None,
name: str | None = None,
Comment thread
Copilot marked this conversation as resolved.
abort_existing: bool = False,
eager_start: bool = True,
log_exceptions: bool = True,
Expand All @@ -752,14 +753,21 @@ def create_task(
:param target: Coroutine function or awaitable to run as a task.
:param args: Arguments to pass to the coroutine function.
:param task_id: Optional ID to track and deduplicate tasks.
:param name: Optional name identifying the task in log messages. Task ids are not
used for this: they key on arguments such as image urls and search terms, which
do not belong in a log line. Keep a name free of those too.
:param abort_existing: If True, cancel existing task with same task_id.
:param eager_start: If True (default), start task immediately without waiting
for next event loop iteration. This ensures proper ordering
when creating multiple tasks in sequence.
:param log_exceptions: Set to False when the caller awaits the task and reports
its failures itself; the task then logs at debug level
instead of warning.
:param kwargs: Keyword arguments to pass to the coroutine function.
:param kwargs: Keyword arguments to pass to the coroutine function. The options
above take these names for themselves, so a coroutine function with a parameter
of its own called task_id, name, abort_existing, eager_start or log_exceptions
has to be called with that argument positionally, or awaited on a coroutine
built by the caller.
"""
if task_id and (existing := self._tracked_tasks.get(task_id)) and not existing.done():
# prevent duplicate tasks if task_id is given and already present
Expand All @@ -783,12 +791,17 @@ def create_task(
else:
raise RuntimeError("Target is missing")

# Use asyncio.Task directly with eager_start for immediate execution
task: asyncio.Task[_R] = asyncio.Task(coro, loop=self.loop, eager_start=eager_start)

if task_id is None:
task_id = uuid4().hex

# asyncio.Task is used directly for eager_start (immediate execution). An eagerly
# started task runs its first step inside the constructor, so the name has to be set
# here: it is what identifies the task in asyncio's own slow-callback warnings and in
# the exception log below. Without one asyncio numbers the task itself.
task: asyncio.Task[_R] = asyncio.Task(
coro, loop=self.loop, eager_start=eager_start, name=name
)

def task_done_callback(_task: asyncio.Task[Any]) -> None:
# done callbacks run one event loop iteration after the task finished, so a
# caller may already have replaced the entry with a new task under the same
Expand Down Expand Up @@ -1479,7 +1492,7 @@ async def _on_provider_loaded() -> None:
if provider.default_name != conf.default_name:
self.config.set_provider_default_name(provider.instance_id, provider.default_name)

self.create_task(_on_provider_loaded())
self.create_task(_on_provider_loaded(), name=f"provider_loaded_{provider.instance_id}")

# clear any previous error in config and signal update
self.config.set(f"{CONF_PROVIDERS}/{conf.instance_id}/last_error", None)
Expand Down
16 changes: 16 additions & 0 deletions tests/core/test_server_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,22 @@ async def _boom() -> None:
)


async def test_create_task_names_only_on_request(mass_minimal: MusicAssistant) -> None:
"""Test that a task is named only by an explicit name, never by its task_id."""

async def _work() -> None:
return

named = mass_minimal.create_task(_work(), name="provider_loaded_some_provider")
assert named.get_name() == "provider_loaded_some_provider"

# task ids key on arguments such as image urls, so they must stay out of the name
keyed = mass_minimal.create_task(_work(), task_id="palette_fetch_p1_current_https://x/y?sig=1")
assert "https://x/y?sig=1" not in keyed.get_name()

await asyncio.gather(named, keyed)


async def test_create_task_replacement_stays_tracked(mass_minimal: MusicAssistant) -> None:
"""Test that a finished task does not untrack a replacement with the same task_id."""
task_id = "test_replacement"
Expand Down
17 changes: 17 additions & 0 deletions tests/helpers/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,16 @@ async def test_failure_reaches_the_caller_without_being_logged(
if record.levelno >= logging.WARNING and "Exception in task" in record.getMessage()
]

@pytest.mark.asyncio
async def test_argument_named_after_a_task_option_reaches_the_wrapped_method(
self, mass_minimal: MusicAssistant
) -> None:
"""A parameter sharing its name with a create_task option is still passed through."""
caller = _GuardedCaller(mass_minimal)
caller.release.set()

assert await caller.fetch_named(name="abc", task_id="123") == "abc-123"

@pytest.mark.asyncio
async def test_instances_get_their_own_request(self, mass_minimal: MusicAssistant) -> None:
"""Two objects of the same class each issue their own request."""
Expand Down Expand Up @@ -908,6 +918,13 @@ async def fetch(self, item_id: str) -> str:
raise self.error
return f"result-{item_id}"

@guard_single_request
async def fetch_named(self, name: str, task_id: str) -> str:
"""Return the arguments, which are named after options of mass.create_task."""
self.calls += 1
await self.release.wait()
return f"{name}-{task_id}"

@guard_single_request
async def fetch_item(
self,
Expand Down