graphene-django icon indicating copy to clipboard operation
graphene-django copied to clipboard

Support atomic mutations for all Mutations

Open cveilleux opened this issue 3 years ago • 3 comments

What is the current behavior?

Exceptions thrown in a graphene.Mutation do not result in a transaction rollback, even though ATOMIC_REQUESTS is on and/or ATOMIC_MUTATIONS is on.

The documentation states:

If the view, a DjangoFormMutation or a DjangoModelFormMutation produces an exception, Django rolls back the transaction

This is an unexpected behavior to me. Many use-cases for mutations do not require a DjangoFormMutation / DjangoModelFormMutation even in a django app, but you would still expect the atomic feature to kick in, especially if ATOMIC_MUTATIONS is on.

What is the expected behavior?

Rollback transactions in all mutations if an unhandled exception is raised by the mutation code when ATOMIC_MUTATIONS or ATOMIC_REQUESTS is set.

Please tell us about your environment:

  • Version: v2.15.0

Workaround

I am currently using the following workaround in order for the rollback on error behavior to kick-in:

class ForceAtomicMutationMiddleware:
    def resolve(self, next, root, info, **args):
        def bound_on_error(error, info=info):
            logger.error("Exception occurred in GraphQL resolver.", exc_info=error)
            # this ensures ALL mutations will rollback, not just DjangoFormMutations
            # see https://docs.graphene-python.org/projects/django/en/latest/mutations/#django-database-transactions
            if info and info.context:
                setattr(info.context, MUTATION_ERRORS_FLAG, True)

        return next(root, info, **args).catch(bound_on_error)

cveilleux avatar Apr 27 '21 14:04 cveilleux

FWIW, this happens on v3 as well.

andrei-datcu avatar Aug 12 '21 22:08 andrei-datcu

+1, I also didn't expect that atomic mutations refers only to DjangoFormMutation/DjangoModelFormMutation. However I have a problem with making a workaround, when I add ForceAtomicMutationMiddleware to GRAPHENE MIDDLEWARES list, using it results in 'QuerySet' object has no attribute 'catch'. If anyone encountered this error and managed to fix it, please let me know!

My workaround is a bit more hacky:

class AtomicSchema(graphene.Schema):
    def execute(self, *args, **kwargs):
        with transaction.atomic():
            result = super().execute(*args, **kwargs)
            if result.errors:
                transaction.set_rollback(True)
            return result

(...)

schema = AtomicSchema(query=Query, mutation=Mutation)

MariuszBielecki288728 avatar Sep 04 '21 23:09 MariuszBielecki288728

I found a workaround which allows controlling the atomicity per-request and will also rollback the transaction on any errors. Here it is possible to control the atomicity by passing a query string parameter ?atomic=1.

class MyGraphQLView(GraphQLView):
    def execute_graphql_request(
        self, request, *args, **kwargs
    ):
        if bool(request.GET.get("atomic", False)):
            with transaction.atomic():
                result = super().execute_graphql_request(request, *args, **kwargs)
                if result and result.errors:
                    transaction.set_rollback(True)
                return result
        else:
            return super().execute_graphql_request(request, *args, **kwargs)

...
urlpatterns = [
    path("graph-api/", MyGraphQLView.as_view(graphiql=False), name="graph-api")
]

syastrov avatar May 10 '22 10:05 syastrov