Transaction committed on domain error for request with multiple mutations

[penwith]

Is there an existing issue for this?

• [X] 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