Skip to content

Conversation

@Natim
Copy link

@Natim Natim commented Dec 17, 2025

What was changed

Make the capture_exception call async in Sentry intercepor

Why?

Use asyncio.to_thread to avoid DeadlockError while sending exception to Sentry

Checklist

  1. Refs feat: add example using Sentry V2 SDK #140

  2. How was this tested:
    In production _DeadlockError was raised

  3. Any docs updates needed?
    No

@Natim Natim requested a review from a team as a code owner December 17, 2025 10:09
@CLAassistant
Copy link

CLAassistant commented Dec 17, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Natim
Copy link
Author

Natim commented Dec 17, 2025

Well, well

  File "/app/project/temporal/interceptor.py", line 129, in execute_workflow
    await asyncio.to_thread(scope.capture_exception, e)

  File "/usr/local/lib/python3.14/asyncio/threads.py", line 25, in to_thread
    return await loop.run_in_executor(None, func_call)
                 ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^

  File "/usr/local/lib/python3.14/asyncio/events.py", line 337, in run_in_executor
    raise NotImplementedError

if not workflow.unsafe.is_replaying():
with workflow.unsafe.sandbox_unrestricted():
scope.capture_exception()
await asyncio.to_thread(scope.capture_exception, e)
Copy link
Member

@cretz cretz Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not legal code from a workflow, and as you saw, is not supported by our event loop (by intention). Two things here...

First, Sentry is on the verge of a 3.x SDK which will include async support (per getsentry/sentry-python#2824 (comment)). They appear to have released a few alphas already.

Second, not sure we should assume they want to use the default executor and you're going to have to use an intentionally-undocumented feature called "unsafe extern" here (we use this for a similar purpose for OTel at https://github.com/temporalio/sdk-python/blob/main/temporalio/contrib/opentelemetry.py). I would suggest doing the following:

  • Have the constructor of SentryInterceptor accept an optional ThreadPoolExecutor, and capture the current event loop as a private attribute on the class
  • Have a def _capture_exception(self, scope: sentry_sdk.Scope, exception: Exception) that calls the loop's run_in_executor with the known thread pool executor for scope's capture_exception
  • Pass the SentryInterceptor to _SentryActivityInboundInterceptor's interceptor so it can use this
  • On workflow_interceptor_class, call input.unsafe_extern_functions.update("__temporal_sentry_capture_exception", self._capture_exception)
  • In _SentryWorkflowInterceptor instead of what's called today, call temporalio.workflow.extern_functions()["__temporal_sentry_capture_exception"](scope, e)

But would understand if there isn't value in doing all of this if it's just going to be outdated shortly with v3 SDK

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.

3 participants