Skip to content

Conversation

Jaza
Copy link

@Jaza Jaza commented Jan 28, 2025

If an async generator dependency raises RuntimeError: generator didn't yield, make solve_dependencies catch and re-raise it, to more easily identify the dependency responsible for the error, and to provide more information on how to fix the dependency.

Copy link
Contributor

@Jaza
Copy link
Author

Jaza commented Jan 28, 2025

This resolves #13238, by making the raised exception more informative, and by adding an example of a badly-behaving async dependency to the docs.

@Jaza Jaza force-pushed the solve-dependencies-generator-didnt-yield branch from 9d6d765 to 99426b4 Compare January 28, 2025 04:23
If an async generator dependency raises RuntimeError:
generator didn't yield, make solve_dependencies catch
and re-raise it, to more easily identify the dependency
responsible for the error, and to provide more information
on how to fix the dependency
@Jaza Jaza force-pushed the solve-dependencies-generator-didnt-yield branch from 99426b4 to efe0e06 Compare January 28, 2025 04:25
Copy link
Contributor

@svlandeg svlandeg added the feature New feature or request label Feb 5, 2025
@svlandeg svlandeg changed the title Fix: make solve_dependencies re-raise RuntimeError: generator didn't yield ✨ Let solve_dependencies re-raise the RuntimeError when a generator didn't yield anything Feb 5, 2025
Copy link
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

@Jaza, thanks for your interest!

I think adding a clear error message is a good idea.
As for updating docs - I think we don't need these code examples. Everything should be clear in current version.

We can just add tests for sync and async dependencies to make sure it works

try:
solved = await stack.enter_async_context(cm)
except RuntimeError as ex:
if "generator didn't yield" not in f"{ex}":
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if "generator didn't yield" not in f"{ex}":
if str(ex) == "generator didn't yield":

This should be faster

Comment on lines +571 to +573
"this is a dependency with yield that has a block with a bare except, or a "
"block with except Exception, and is not raising the exception again. Read "
"more about it in the docs: "
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"this is a dependency with yield that has a block with a bare except, or a "
"block with except Exception, and is not raising the exception again. Read "
"more about it in the docs: "
"this is a dependency with yield that catches an exception using except, "
"but doesn't raise the exception again. Read more about it in the docs: "

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request waiting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants