Skip to content
Merged
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
11 changes: 11 additions & 0 deletions architecture/dependency-injection.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ opt into injection by annotating them
Celery's own argument binding reads the rewritten signature and only
expects the caller's real arguments, never the DI ones.

Resolution binds the caller's arguments to the visible signature *by name*
(`bound.arguments`), which is what makes injection parameter-order-insensitive.
That by-name call cannot faithfully forward `*args`/`**kwargs`: `Signature.bind`
stores their values under the literal names `"args"`/`"kwargs"`, so
`func(**bound.arguments, ...)` would misroute a variadic payload into a keyword
argument. Rather than silently corrupt arguments, `inject` **rejects at
decoration time** (raises `TypeError`) any task that declares a `VAR_POSITIONAL`
or `VAR_KEYWORD` parameter *alongside* a `FromDI` parameter. A task with no
`FromDI` parameter is returned unchanged (step 2) and may use `*args`/`**kwargs`
freely.

## DITask

`DITask(Task)` is the auto-inject path, used via `task_cls=DITask` on the
Expand Down
8 changes: 8 additions & 0 deletions modern_di_celery/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ def inject(func: typing.Callable[..., T]) -> typing.Callable[..., T]:
return func

signature = inspect.signature(func)
for name, param in signature.parameters.items():
if param.kind in (inspect.Parameter.VAR_POSITIONAL, inspect.Parameter.VAR_KEYWORD):
func_name = getattr(func, "__qualname__", repr(func))
msg = (
f"@inject task {func_name!r} declares *args/**kwargs (parameter {name!r}), "
"which is unsupported; use explicit named parameters instead of *args/**kwargs with @inject."
)
raise TypeError(msg)
visible_params = [p for name, p in signature.parameters.items() if name not in di_params]
visible_signature = signature.replace(parameters=visible_params)

Expand Down
33 changes: 33 additions & 0 deletions planning/releases/2.0.1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# modern-di-celery 2.0.1 — `@inject` variadic fail-fast

A patch release fixing silent argument corruption in `@inject`.

## Fix

- **`@inject` now rejects `*args`/`**kwargs` at decoration time.** Since 2.0.0,
`@inject` binds a task's arguments to its visible signature *by name* (which is
what makes injection parameter-order-insensitive). That by-name call could not
faithfully forward variadic parameters: `inspect.Signature.bind` stores
`*args`/`**kwargs` values under the literal names `"args"`/`"kwargs"`, so a
task declaring `*args`/`**kwargs` **alongside** a `FromDI` parameter had its
real payload silently misrouted into a keyword argument (e.g. a caller's
`(1, 2, foo="bar")` arrived inside the task as `args=(), kwargs={"args": (1, 2),
"kwargs": {"foo": "bar"}}`) instead of raising.

`@inject` now raises a clear `TypeError` at decoration time when a task
declares a `VAR_POSITIONAL` or `VAR_KEYWORD` parameter together with a `FromDI`
parameter, naming the offending parameter and pointing at explicit named
parameters as the fix. A task with **no** `FromDI` parameter is returned
unchanged and may use `*args`/`**kwargs` freely.

## Downstream

- **Potentially breaking, intentionally.** A task that previously combined
`@inject`/`DITask` with `*args`/`**kwargs` and a `FromDI` parameter was already
broken at runtime (silently corrupted arguments); it now fails loudly at import
time. Replace the variadic parameters with explicit named parameters.

## Internals

- 100% line coverage; `ruff`, `ty`, and `eof-fixer` clean. Two new tests cover
the `VAR_POSITIONAL` and `VAR_KEYWORD` rejection paths.
22 changes: 22 additions & 0 deletions tests/test_inject.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,25 @@ def sample(_res: typing.Annotated[SimpleCreator, FromDI(Boom.resource)]) -> None
with pytest.raises(ValueError, match="boom"):
sample.delay().get()
assert teardowns == ["closed"] # per-task child closed (finalizer ran) on the error path


def test_inject_rejects_var_positional_with_fromdi() -> None:
def bad_task(
_svc: typing.Annotated[SimpleCreator, FromDI(SimpleCreator)],
*args: int,
) -> tuple[int, ...]:
return args # pragma: no cover

with pytest.raises(TypeError):
inject(bad_task)


def test_inject_rejects_var_keyword_with_fromdi() -> None:
def bad_task(
_svc: typing.Annotated[SimpleCreator, FromDI(SimpleCreator)],
**kwargs: int,
) -> dict[str, int]:
return kwargs # pragma: no cover

with pytest.raises(TypeError):
inject(bad_task)