graphql-dotnet / conventions

GraphQL Conventions Library for .NET
MIT License
233 stars 63 forks source link

Does "WithExposedExceptions" work as expected? #181

Closed shoe-diamente closed 4 years ago

shoe-diamente commented 5 years ago

I have set:

engine.WithExposedExceptions(false);

but exceptions from resolvers (that are not ExecutionErrors) are still exposed. Specifically, a System.NotSupported exception is exposed as:

{
  "data": ...,
  "errors": [
    {
      "message": "Specified method is not supported.",
      "locations": [
        {
          "line": 3,
          "column": 5
        }
      ],
      "path": [...],
      "extensions": {
        "code": "NOT_SUPPORTED"
      }
    }
  ]
}

I'm returning the result via:

return enging.SerializeResult(result);

What am I missing? How can I prevent exceptions that are not ExecutionErrors from being exposed?

tlil commented 5 years ago

The exception will be exposed, but the stacktrace for said exception won't. See test/Tests/Adapters/Engine/ExposingExceptionsTests.cs for examples. Hope that helps.

shoe-diamente commented 5 years ago

Oh, I see. Is there any way to customise the error in case of unrecognised exceptions or to block them off entirely?

sungam3r commented 5 years ago

In graphql-dotnet you can use ExecutionOptions.UnhandledExceptionDelegate.

tlil commented 5 years ago

You could also create an execution filter and throw a try-catch block around the execution handler :) That way you can control it 100%.

shoe-diamente commented 5 years ago

@sungam3r Hi thanks, but it doesn't seem possible to set those options in graphql-dotnet/conventions. Am I wrong?

shoe-diamente commented 5 years ago

@tlil Neat. I can't seem to find examples of that in the tests. Is that documented anywhere? Where could I see an example?

sungam3r commented 5 years ago

it doesn't seem possible to set those options in graphql-dotnet/conventions

I guessed about it. I have not watched the conventions project yet, although I can roughly imagine what it is. graphql-dotnet gradually evolves and new features appear in it, the reflection of which should somehow appear in consuming projects.

tlil commented 5 years ago

@tlil Neat. I can't seem to find examples of that in the tests. Is that documented anywhere? Where could I see an example?

Take a look at ChaosAttribute.cs, @shoe-diamente :-)

shoe-diamente commented 5 years ago

@tlil In that example should then attach [ChaosMetaData] to every field, or...?

tlil commented 5 years ago

If you mark it as IDefaultAttribute, it will be on by default :)

shoe-diamente commented 5 years ago

I've tried with:

    public class ChaosAttribute : ExecutionFilterAttributeBase
    {
        public override Task<object> Execute(IResolutionContext context, FieldResolutionDelegate next)
        {
            throw new Exception("WWWWW");
            return next(context);
        }
    }

    public class ChaosMetaDataAttribute : MetaDataAttributeBase, IDefaultAttribute
    {
        public override void MapField(GraphFieldInfo fieldInfo, MemberInfo memberInfo)
        {
            fieldInfo.ExecutionFilters.Add(new ChaosAttribute());
        }
    }

but unless I manually add it via [ChaosMetaData], it won't throw exceptions anywhere. I'm missing something.

Do I have to register it somewhere? Can I inject services with the native .NET Core dependency injection system?

shoe-diamente commented 5 years ago

It doesn't pick it up because the method that discovers them only discovers them in the assembly of the library. There doesn't seem to be a way to specify a different assembly.

shoe-diamente commented 4 years ago

I tried with:

var result = await _engine
                .WithExposedExceptions(false)
                .WithAttributesFromAssembly<Graph.ErrorsGuardMetaDataAttribute>()
                .NewExecutor()
                .WithUserContext(userContext)
                .WithDependencyInjector(_injector)
                .WithRequest(requestBody)
                .WithCancellationToken(cancellationToken)
                .Execute();

where Graph.ErrorsGuardMetaDataAttribute is:

    public class ErrorsGuardMetaDataAttribute : MetaDataAttributeBase, IDefaultAttribute
    {
        public override void MapField(GraphFieldInfo fieldInfo, MemberInfo memberInfo)
        {
            fieldInfo.ExecutionFilters.Add(new ErrorsGuardAttribute());
        }
    }

Then following the code in this repository I tried to simulate the "search of default attributes" with that method with:

            var assembly = typeof(Graph.ErrorsGuardMetaDataAttribute).GetTypeInfo().Assembly;
            var defaultAttributes = assembly
                .GetTypes()
                .Where(type => IsDefaultAttribute(type) && IsTAttribute(type))
                .ToArray();

and:

        private bool IsDefaultAttribute(Type type)
        {
            return type.GetTypeInfo()
                .GetInterfaces()
                .Any(iface => iface == typeof(IDefaultAttribute));
        }

        private bool IsTAttribute(Type type)
        {
            return type.GetTypeInfo()
                .GetInterfaces()
                .Any(iface => iface == typeof(IMetaDataAttribute));
        }

and the array contains 1 element: Graph.ErrorsGuardMetaDataAttribute. Which implies that that metadata attribute is correctly found.

But when I try to run a query nothing of Graph.ErrorsGuardMetaDataAttribute nor ErrorsGuardAttribute gets executed and exceptions other than AppError go through.

What am I doing wrong?

shoe-diamente commented 4 years ago

Even debugging the engine I get: Screen Shot 2019-11-15 at 17 02 02

And as you can see the ErrorsGuardMetaDataAttribute gets correctly registered as a default attribute. But it doesn't actually get executed ever.

tlil commented 4 years ago

I missed this response, sorry. If you're still struggling to get it working, please re-open with a repro case that I can look at.