You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@burr.apache.org by "Eilen6316 (via GitHub)" <gi...@apache.org> on 2026/08/20 08:47:11 UTC
[PR] fix: report execute hook failures [burr]
Eilen6316 opened a new pull request, #897:
URL: https://github.com/apache/burr/pull/897
## Summary
- pass the original exception, including cancellation, to top-level execute post hooks
- defer execute hook completion for generator and async-generator APIs until iteration ends
- clear execution hook state when pre hooks or streaming setup fail
- report the correct `astream_result` execute method
## Testing
- `uv run --with pytest --with pytest-asyncio --with aiosqlite --with 'pydantic[email]>=2.0' python -m pytest tests/core --ignore=tests/core/test_graphviz_display.py -q`\n- `uv run --with 'opentelemetry-api' --with 'opentelemetry-sdk' --with 'langfuse>=3.0.0' --with pytest --with pytest-asyncio --with 'pydantic[email]>=2.0' python -m pytest tests/integrations/test_opentelemetry.py tests/integrations/test_burr_opentelemetry.py tests/integrations/test_burr_langfuse.py -q`\n- `black --check` and `flake8` on modified Python files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@burr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: report execute hook failures [burr]
Posted by "sangkyoonnam (via GitHub)" <gi...@apache.org>.
sangkyoonnam commented on code in PR #897:
URL: https://github.com/apache/burr/pull/897#discussion_r4176183225
##########
burr/core/application.py:
##########
@@ -746,23 +754,74 @@ def __call__(self, fn: CallableT) -> CallableT:
@functools.wraps(fn)
async def wrapper_async(app_self, *args, **kwargs):
# We only run at the top level, so we decorate it if we're there
- await self.acall_pre(app_self)
- exc = None
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
try:
+ should_run_hooks = await self.acall_pre(app_self)
return await fn(app_self, *args, **kwargs)
+ except BaseException as e:
+ exc = e
+ raise
finally:
- await self.acall_post(app_self, exc)
+ if should_run_hooks:
+ await self.acall_post(app_self, exc)
@functools.wraps(fn)
def wrapper_sync(app_self, *args, **kwargs):
- self.call_pre(app_self)
- exc = None
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
try:
+ should_run_hooks = self.call_pre(app_self)
return fn(app_self, *args, **kwargs)
+ except BaseException as e:
+ exc = e
+ raise
finally:
- self.call_post(app_self, exc)
+ if should_run_hooks:
+ self.call_post(app_self, exc)
- return wrapper_async if inspect.iscoroutinefunction(fn) else wrapper_sync
+ @functools.wraps(fn)
+ def wrapper_generator(app_self, *args, **kwargs):
+ def generator():
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
+ try:
+ should_run_hooks = self.call_pre(app_self)
Review Comment:
The sentinel set here stays in the caller's context while the generator is suspended, so other calls on the same app skip their execute hooks.
```python
gen = app.iterate(halt_after=["c"])
next(gen)
app.step()
```
Expected `pre`/`post` for `step` (main does that). Observed none. `aiterate` plus `astep()` and an open `iterate` plus `app.run()` behave the same way. One option: set `_run_call_var` around each `next(inner)` and reset it before yielding. `call_post` checks for the matching sentinel, so the final post call would need it set again for that call.
##########
burr/core/application.py:
##########
@@ -746,23 +754,74 @@ def __call__(self, fn: CallableT) -> CallableT:
@functools.wraps(fn)
async def wrapper_async(app_self, *args, **kwargs):
# We only run at the top level, so we decorate it if we're there
- await self.acall_pre(app_self)
- exc = None
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
try:
+ should_run_hooks = await self.acall_pre(app_self)
return await fn(app_self, *args, **kwargs)
+ except BaseException as e:
+ exc = e
+ raise
finally:
- await self.acall_post(app_self, exc)
+ if should_run_hooks:
+ await self.acall_post(app_self, exc)
@functools.wraps(fn)
def wrapper_sync(app_self, *args, **kwargs):
- self.call_pre(app_self)
- exc = None
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
try:
+ should_run_hooks = self.call_pre(app_self)
return fn(app_self, *args, **kwargs)
+ except BaseException as e:
+ exc = e
+ raise
finally:
- self.call_post(app_self, exc)
+ if should_run_hooks:
+ self.call_post(app_self, exc)
- return wrapper_async if inspect.iscoroutinefunction(fn) else wrapper_sync
+ @functools.wraps(fn)
+ def wrapper_generator(app_self, *args, **kwargs):
+ def generator():
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
+ try:
+ should_run_hooks = self.call_pre(app_self)
+ return (yield from fn(app_self, *args, **kwargs))
+ except BaseException as e:
+ exc = e
+ raise
+ finally:
+ if should_run_hooks:
+ self.call_post(app_self, exc)
+
+ return generator()
+
+ @functools.wraps(fn)
+ def wrapper_async_generator(app_self, *args, **kwargs):
+ async def generator():
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
+ try:
+ should_run_hooks = await self.acall_pre(app_self)
+ async for item in fn(app_self, *args, **kwargs):
Review Comment:
With `async for ... break` and no `aclose()`, asyncio finalizes the generator in a separate task, so the cleanup never reaches the caller's context.
```python
async for _ in app.aiterate(halt_after=["c"]):
break
gc.collect()
for _ in range(3):
await asyncio.sleep(0)
await app.astep()
```
The later `astep()` runs no execute hooks. With the OTel bridge, OTel logs `Failed to detach context` (`ValueError: ... created in a different Context`), no `astep` span is emitted, and the action span is parented to the already-ended `aiterate` span. On main both `astep` calls get their own root span. Scoping the sentinel per resume would likely fix the hook side. I haven't verified a fix for the OTel side.
##########
burr/core/application.py:
##########
@@ -746,23 +754,74 @@ def __call__(self, fn: CallableT) -> CallableT:
@functools.wraps(fn)
async def wrapper_async(app_self, *args, **kwargs):
# We only run at the top level, so we decorate it if we're there
- await self.acall_pre(app_self)
- exc = None
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
try:
+ should_run_hooks = await self.acall_pre(app_self)
return await fn(app_self, *args, **kwargs)
+ except BaseException as e:
+ exc = e
+ raise
finally:
- await self.acall_post(app_self, exc)
+ if should_run_hooks:
+ await self.acall_post(app_self, exc)
@functools.wraps(fn)
def wrapper_sync(app_self, *args, **kwargs):
- self.call_pre(app_self)
- exc = None
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
try:
+ should_run_hooks = self.call_pre(app_self)
return fn(app_self, *args, **kwargs)
+ except BaseException as e:
+ exc = e
+ raise
finally:
- self.call_post(app_self, exc)
+ if should_run_hooks:
+ self.call_post(app_self, exc)
- return wrapper_async if inspect.iscoroutinefunction(fn) else wrapper_sync
+ @functools.wraps(fn)
+ def wrapper_generator(app_self, *args, **kwargs):
+ def generator():
+ should_run_hooks = False
+ exc: Optional[BaseException] = None
+ try:
+ should_run_hooks = self.call_pre(app_self)
+ return (yield from fn(app_self, *args, **kwargs))
+ except BaseException as e:
Review Comment:
`GeneratorExit` lands here, so a normal early exit is reported as a failure.
```python
for _ in app.iterate(halt_after=["c"]):
break
```
On main the root `iterate` span is `OK`. On this branch it's `ERROR`, and `post_run_execute_call` gets `exception=GeneratorExit`. `aiterate` followed by `aclose()` does the same. Suggest `except GeneratorExit: raise` ahead of this clause so `exc` stays `None`; same at line 809.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@burr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: report execute hook failures [burr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #897:
URL: https://github.com/apache/burr/pull/897#issuecomment-6266921531
This PR has been inactive for 15 days after receiving review feedback.
Converting to draft.
Please mark it as ready for review when you have addressed the comments.
If no activity occurs within 90 days, it will be closed automatically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@burr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org