search

ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.26k stars 746 forks source link

Exceptions from diagnostic event listeners prevent query execution #4412

Closed vhatsura closed 3 years ago

vhatsura commented 3 years ago

Is there an existing issue for this?

Describe the bug

In case of a diagnostic event, the listener throws an exception for some reason, HC will not execute the query, and no information about an error will be returned or logged.

Steps to reproduce

Currently, the issue is reproducible with the ElasticApm.GraphQL.HotChocolate NuGet package with 2.0.0-preview.1 version. The details of the problem can be found at https://github.com/SwissLife-OSS/elastic-apm-extensions/issues/14.

I expect that any errors from event listeners do not prevent HC from query execution. Logging or exposing somehow an error will also be beneficial to spot an issue.

Relevant log output

No response

Additional Context?

No response

Product

Hot Chocolate

Version

12.0.0 and higher

michaelstaib commented 3 years ago

The diagnostic listeners are not allowed to throw any exceptions. Its up to the implementer of listeners to ensure this.

vhatsura commented 3 years ago

The diagnostic listeners are not allowed to throw any exceptions. Its up to the implementer of listeners to ensure this.

I agree with you that listeners should take care about what and how they do some things. However, as diagnostic events in HC are made as a synchronous mechanism, I don't think it's fair to return an empty object as a query result in case of any failures in the diagnostic event listener. There is no indicator that execution was failed