Skip to content

This fixes embedded async resolvers for Ariadne #127

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

Closed
wants to merge 1 commit into from

Conversation

ammurdoch
Copy link

I ran into an issue while using Ariadne where embedded async resolvers were not getting awaited and causing my queries to fail. Top level async resolvers worked fine, but resolvers two or three levels down weren't getting awaited. I dug into the issue and was able to fix it by adding the following to the resolve_field_value_or_error function:

def resolve_field_value_or_error(
        self,
        field_def: GraphQLField,
        field_nodes: List[FieldNode],
        resolve_fn: GraphQLFieldResolver,
        source: Any,
        info: GraphQLResolveInfo,
    ) -> Union[Exception, Any]:
        """Resolve field to a value or an error.

        Isolates the "ReturnOrAbrupt" behavior to not de-opt the resolve_field()
        method. Returns the result of resolveFn or the abrupt-return Error object.

        For internal use only.
        """
        try:
            # Build a dictionary of arguments from the field.arguments AST, using the
            # variables scope to fulfill any variable references.
            args = get_argument_values(field_def, field_nodes[0], self.variable_values)

            # Note that contrary to the JavaScript implementation, we pass the context
            # value as part of the resolve info.
            result = resolve_fn(source, info, **args)
            if self.is_awaitable(result):
                # noinspection PyShadowingNames
                async def await_result() -> Any:
                    try:
-                        return await result
+                        intermediate_result = await result
+                        if isawaitable(intermediate_result):
+                             return await intermediate_result
+                        return intermediate_result
                    except Exception as error:
                        return error

                return await_result()
            return result
        except Exception as error:
            return error

@ammurdoch ammurdoch requested a review from Cito as a code owner April 9, 2021 03:55
Copy link
Member

@Cito Cito left a comment

Choose a reason for hiding this comment

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

  • Can you provide an executable example (that doesn't involved Ariadne)? You don't need to make an actual unit test from this, I can do this.
  • It looks like this works only for two levels of nesting - if we do something like this, it should work for deeper nesting as well.
  • The actual problem may be that an await is missing in the nested resolvers. Therefore we really need some example code.

@Cito Cito force-pushed the main branch 3 times, most recently from 4f69c9b to 1804bd7 Compare September 29, 2021 15:09
@Cito Cito force-pushed the main branch 3 times, most recently from 0ac79de to 3f40fc2 Compare November 7, 2021 20:26
@Cito Cito added pending Waiting for other issues to move forward question Further information is requested labels Dec 28, 2021
@Cito
Copy link
Member

Cito commented Jan 15, 2022

Closing this since there was no more feedback. If you still think there is a problem, please open a new issue with a reproducable example.

@Cito Cito closed this Jan 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending Waiting for other issues to move forward question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants