Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When adding an IErrorFilter with dependencies, that is created using the provided ServiceFactory of .AddErrorFilter, the ErrorFilter is never instantiated, nor executed. #2768

Closed
fredericbirke opened this issue Dec 19, 2020 · 6 comments
Assignees
Milestone

Comments

@fredericbirke
Copy link
Contributor

Describe the bug

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddGraphQLServer()
                .AddErrorFilter(factory => new MyErrorFilter(factory.GetApplicationService<ILogger<MyErrorFilter>>()))
                .AddQueryType<Query>();
        }
public class MyErrorFilter: IErrorFilter
    {
        private readonly ILogger<MyErrorFilter> _myErrorFilter;
        public MyErrorFilter(ILogger<MyErrorFilter> myErrorFilter)
        {
            _myErrorFilter = myErrorFilter;
        }
        public IError OnError(IError error)
        {
            Console.WriteLine("An Error occured!");
            return error;
        }
    }

To Reproduce
Checkout https://github.com/fredericbirke/HcErrorFilterBug, start it and fire the ping query using BCP.
If you set the breakpoints, you will see, that the ErrorFilter is never instantiated or used.

Expected behavior
The ErrorFilter should work exactly the same way as the ErrorFilter, that has no dependencies to other services from the DI-Container.

@shgreig-mtm
Copy link

Does anyone have any way of working around this issue.
I'm trying to get an ILogger into my Error filter so I can log all errors that come from the api but obviously can't inject the ILogger at the moment as the API breaks when I add this in

@PascalSenn
Copy link
Member

@shgreig-mtm this should be fixed with #2774. Do you still see this issue? What version are you on?

@shgreig-mtm
Copy link

shgreig-mtm commented Jan 12, 2022

@PascalSenn 12.4.1
So if I do just this

public class ErrorFilter : IErrorFilter
{
	private readonly ILogger<ErrorFilter> _logger;
	private readonly IHttpContextAccessor _contextAccessor;

	public ErrorFilter()
	{
	
	}
	public IError OnError(IError error)
	{
            return error.WithMessage(error.Exception.Message);
	}
}

Its working

But when I try and inject into the constructor I all of my queries will just 500 error

public class ErrorFilter : IErrorFilter
{
	private readonly ILogger<ErrorFilter> _logger;
	private readonly IHttpContextAccessor _contextAccessor;

	public ErrorFilter(ILogger<ErrorFilter> logger, HttpContextAccessor httpContextAccessor)
	{
		_contextAccessor = httpContextAccessor;
		_logger = logger;
	}
	public IError OnError(IError error)
	{
		return error.WithMessage(error.Exception.Message);
	}
}

@PascalSenn
Copy link
Member

@shgreig-mtm You have to register it with the delegate overload:

 .AddErrorFilter(factory => new ErrorFilter(factory.GetApplicationService<ILogger< ErrorFilter>>(),....))
               

@shgreig-mtm
Copy link

Ah okay thank you, that is now working

@PascalSenn
Copy link
Member

Cool :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants