rivantsov / ngraphql

GraphQL .NET Server and Client
MIT License
43 stars 5 forks source link

Error format in NGraphQL #20

Closed jasonlaw closed 3 years ago

jasonlaw commented 3 years ago

Hi @rivantsov ,

I have a Entity validation logic as below.

        [Entity]
        [Validate(typeof(AuthorizationModule), nameof(AuthorizationModule.ValidateLogin))]
        [EntitySavingEvent(typeof(AuthorizationModule), nameof(AuthorizationModule.SavingLogin))]
        [Display("{LoginId}/{Username}")]
        public interface ILogin
        {
           ...
        }

        public static void ValidateLogin(ILogin login)
        {
            var record = EntityHelper.GetRecord(login);
            if (record.Status == EntityStatus.New)
            {
                var session = record.Session;
                var existing = session.FindLogin(login.Username, login.Tenant);
                session.ValidateEntity(login, existing == null, "", "", null, $"Login with username {login.Username} already exists.");
            }
        }

When calling from REST, I have the error formatted as below.

[
  {
    "code": "",
    "message": "Login with username jason.cclaw@gmail.com already exists.",
    "tag": "",
    "path": "ILogin/ILogin/01F6PX8W98JTDD05XNDCQC0YN8",
    "parameters": {}
  }
]

However, when calling from NGraphQL, I have the following error format:

{
  "errors": [
    {
      "message": "Client faults detected.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "register"
      ],
      "extensions": {
        "code": "RESOLVER_ERROR",
        "Details": "Vita.Entities.ClientFaultException: Client faults detected.\r\n   at Vita.Entities.OperationContext.ThrowValidation()\r\n   at Vita.Entities.Runtime.EntitySession.SaveChanges()\r\n   at Aether.Booking.BookingModule.RegisterCustomer(OperationContext context, RegisterCustomerInput input) in E:\\JSL\\Aether\\aetherall\\Aether.Booking\\BookingModule_Register.cs:line 73\r\n   at Aether.Booking.Api.GraphQL.BookingResolvers.Register(IFieldContext context, RegisterCustomerInput input) in E:\\JSL\\Aether\\aetherall\\Aether.Booking.Api\\GraphQL\\BookingResolvers_Mutation_Register.cs:line 12\r\nFaults:\r\nLogin with username jason.cclaw@gmail.com already exists.\r\n"
      }
    },
    {
      "message": "Client faults detected.",
      "locations": [],
      "path": [],
      "extensions": {
        "code": "SERVER_ERROR",
        "Details": "Vita.Entities.ClientFaultException: Client faults detected.\r\n   at Vita.Entities.OperationContext.ThrowValidation()\r\n   at Vita.Entities.Runtime.EntitySession.SaveChanges()\r\n   at Aether.Booking.Api.GraphQL.BookingResolvers.EndRequest(IRequestContext request) in E:\\JSL\\Aether\\aetherall\\Aether.Booking.Api\\GraphQL\\BookingResolvers.cs:line 25\r\n   at NGraphQL.Server.Execution.OperationFieldExecuter.ExecuteOperationFieldAsync()\r\n   at NGraphQL.Server.Execution.RequestHandler.ExecuteAllNonParallel(IList`1 executers)\r\n   at NGraphQL.Server.Execution.RequestHandler.ExecuteOperationAsync(GraphQLOperation op, OutputObjectScope topScope)\r\n   at NGraphQL.Server.Execution.RequestHandler.ExecuteAsync()\r\n   at NGraphQL.Server.GraphQLServer.ExecuteRequestAsync(RequestContext context)\r\nFaults:\r\nLogin with username jason.cclaw@gmail.com already exists.\r\n  Login with username jason.cclaw@gmail.com already exists.\r\n"
      }
    }
  ],
  "data": {}
}

Do you think it is possible to make both error format in a more consistent way, at least the message in GraphQL should be "Login with username jason.cclaw@gmail.com already exists.".

rivantsov commented 3 years ago

yes, you are right, it should show up better, just requires some integration between the two fworks; NGraphQL does not know about ClientFaultException from Vita and treats it as general exc. I am almost done with refactoring, will push new version within days, will put a fix for this and other issues there.

rivantsov commented 3 years ago

added facilities to support this, see ThingsGraphQLServer.cs: https://github.com/rivantsov/ngraphql/blob/master/src/TestApp/Things.GraphQL/ThingsGraphQLServer.cs#L26

it uses AggregateExc as an example - how to convert inner exceptions into errors in response. There's a unit test for this. You can do the same with ClientFaultException. Will add this to GraphQL sample in VITA later

jasonlaw commented 3 years ago

It is working, thanks!