Transaction committed on domain error for request with multiple mutations

[penwith]

Is there an existing issue for this?

• I have searched the existing issues

Product

Hot Chocolate

Describe the bug

Running multiple mutations in a single request. As per the docs here, I want to wrap these in a transaction.

• When one of my mutations is not executed because of an Authorization failure, the transaction is not committed, as I would expect.
• When one of my mutations is not executed because of an unexpected Exception, say a database timeout, the transaction is not committed, as I would expect.
• When one of my mutations returns a domain Error, as exposed to the schema, the transaction is committed, but should not be.

Surely the transaction should be discarded when returning a domain error from one of the mutations in a request?

Steps to reproduce

1. Configure Hot Chocolate GraphQL server with default transaction scope handler and mutation conventions:

builder.Services
	.AddGraphQLServer()
	.AddAuthorization()
	.AddDefaultTransactionScopeHandler()
	.AddTypes()
	.AddMutationConventions(new MutationConventionOptions
	{
		InputArgumentName = "input",
		InputTypeNamePattern = "{MutationName}Input",
		PayloadTypeNamePattern = "{MutationName}Payload",
		PayloadErrorTypeNamePattern = "{MutationName}Error",
		PayloadErrorsFieldName = "errors",
		ApplyToAllMutations = true
	});

2. Add mutation resolvers, and cause one to throw a domain error

[MutationType]
public class Mutations
{
	[Error(typeof(ArgumentOutOfRangeException))]
	public async Task<AffectedRowsResponse> UpdateFirstAsync(
		UpdateModel model,
		CancellationToken cancellationToken)
	{
		return new AffectedRowsResponse() { AffectedRows = 11 };
	}

	[Error(typeof(ArgumentOutOfRangeException))]
	public async Task<AffectedRowsResponse> UpdateSecondAsync(
		UpdateModel model,
		CancellationToken cancellationToken)
	{
		throw new ArgumentOutOfRangeException("Some error");
		return new AffectedRowsResponse() { AffectedRows = 22 };
	}
}

3. Run a request with multiple mutations

mutation TransactionScopeTest (
  $firstInput: UpdateFirstInput!,
  $secondInput: UpdateSecondInput!)
  {
    updateFirst(input: $firstInput)
    {
      affectedRowsResponse {affectedRows}
    }
    updateSecond(input: $secondInput)
    {
      affectedRowsResponse {affectedRows}
    }
  }

4. Transaction is committed

Relevant log output

No response

Additional Context?

No response

Version

13.7.0

[michaelstaib]

I had a stab at this one and we have a rough idea but it also has a lot of implications. To stop the mutation flow here we actually need to throw an error for the other mutations ... a GraphQL error to comply with the error flows. We can do that but I am not really happy with that ... I will discuss that with a couple of people to get some feedback.

[michaelstaib]

I am moving it to 13.9

[dominique-ruest]

Ran into the same issue.

As a workaround I added a custom transaction handler that returns this "CustomTransactionScope" which inspects result objects.

public class CustomTransactionScope : ITransactionScope
{
    public CustomTransactionScope(IRequestContext context, TransactionScope transaction)
    {
        Context = context;
        Transaction = transaction;
    }

    protected IRequestContext Context { get; }

    protected TransactionScope Transaction { get; }

    public void Complete()
    {
        if (Context.Result is OperationResult { Data: not null, Errors: null or { Count: 0, }, })
        {
            var operationResultData = ((OperationResult)Context.Result).Data;
            if (operationResultData != null && operationResultData
                .Where(p => p.Value is ObjectResult)
                .All(p => (((ObjectResult)p.Value!).GetValueOrDefault("errors") as ListResult)?.Count <= 0))
            {
                Transaction.Complete();
            }
        }
    }

    public void Dispose()
    {
        Transaction.Dispose();
    }
}