Skip to content

fix: async interceptor should_intercept must be sync + temporal example fixes#734

Merged
skrawcz merged 1 commit intoapache:stefan/add-more-lifecyclesfrom
andreahlert:fix/pr-602-interceptors
Apr 10, 2026
Merged

fix: async interceptor should_intercept must be sync + temporal example fixes#734
skrawcz merged 1 commit intoapache:stefan/add-more-lifecyclesfrom
andreahlert:fix/pr-602-interceptors

Conversation

@andreahlert
Copy link
Copy Markdown
Collaborator

Summary

Fixes bugs in the interceptor lifecycle hooks introduced in #602:

  • should_intercept on async interceptor classes was declared async def, but callers use it inside lambdas passed to get_first_matching_hook() which cannot await. An async should_intercept returns a coroutine object (always truthy), silently matching every action regardless of the predicate logic. Changed to def with a note explaining why.
  • StreamingActionInterceptorHookAsync.intercept_stream_run_and_update was def but the caller checks isasyncgenfunction, so it needs to be async def to match.
  • Temporal example called worker_adapter_set.get_hooks() which doesn't exist on LifecycleAdapterSet. Replaced with call_all_lifecycle_hooks_async().
  • Temporal worker hook signatures used str for action and **kwargs instead of Action and **future_kwargs.

Test plan

  • All 311 tests pass (core + lifecycle + interceptor integration tests)
  • No changes to test files needed (tests already used sync should_intercept)
  • Verified no remaining async def should_intercept in codebase

The async interceptor base classes (ActionExecutionInterceptorHookAsync,
StreamingActionInterceptorHookAsync) declared should_intercept as async,
but callers use it inside lambdas that cannot be awaited. An async
should_intercept would return a coroutine object (always truthy),
silently matching every action regardless of the predicate logic.

Also fixes:
- StreamingActionInterceptorHookAsync.intercept_stream_run_and_update
  now correctly declared as async (matches isasyncgenfunction check)
- Temporal example: replace nonexistent get_hooks() with
  call_all_lifecycle_hooks_async(), fix worker hook signatures
@skrawcz skrawcz merged commit 9753687 into apache:stefan/add-more-lifecycles Apr 10, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants