Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(product-assistant): correct exception to stop the async iterator #27101

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

skoob13
Copy link
Contributor

@skoob13 skoob13 commented Dec 20, 2024

Problem

An async iterator must throw the StopAsyncIteration exception instead of a regular StopIteration. Currently, it doesn't happen, so the stream stays open until it gets killed by a timeout (my best guess now). After this fix, I haven't observed this issue locally on the ASGI webserver.

Related Sentry log.

Changes

  • Implement a sync to async iterator conversion based on an asgiref implementation.
  • Catch StopIteration and raise StopAsyncIteration, so the sync_to_async of asgiref can stop the iterator.

Does this work well for both Cloud and self-hosted?

No

How did you test this code?

Unit tests.

@skoob13 skoob13 marked this pull request as ready for review December 20, 2024 14:52
@skoob13 skoob13 requested a review from Twixes December 20, 2024 14:53
@PostHog PostHog deleted a comment from posthog-bot Dec 20, 2024
@skoob13 skoob13 enabled auto-merge (squash) December 20, 2024 15:06
Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

Let's see in prod

@skoob13 skoob13 merged commit c249ecd into master Dec 20, 2024
99 checks passed
@skoob13 skoob13 deleted the fix/async-iterator-exception branch December 20, 2024 17:30
Copy link

sentry-io bot commented Dec 20, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ GraphRecursionError: Recursion limit of 24 reached without hitting a stop condition. You can increase the limit by set... /api/environments/{parent_lookup_team_id}/conve... View Issue
  • ‼️ GraphRecursionError: Recursion limit of 24 reached without hitting a stop condition. You can increase the limit by set... ee.hogai.assistant in _stream View Issue
  • ‼️ ValueError: Can only run summarization with a visualization message as the last one in the state ee.hogai.summarizer.nodes in run View Issue
  • ‼️ ValueError: Can only run summarization with a visualization message as the last one in the state /api/environments/{parent_lookup_team_id}/conve... View Issue

Did you find this useful? React with a 👍 or 👎

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