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

Uneccessary execution error wrapping #120

Closed
BilyachenkoOY opened this issue Feb 7, 2019 · 4 comments
Closed

Uneccessary execution error wrapping #120

BilyachenkoOY opened this issue Feb 7, 2019 · 4 comments
Assignees
Labels

Comments

@BilyachenkoOY
Copy link
Contributor

BilyachenkoOY commented Feb 7, 2019

After rewriting my original solution using GraphQL.Conventions I fall into the issue that I can't handle errors as easily as before. For example, this code is not working anymore:

// filtering by type
var criticalErros = result.Errors?.Except(result.Errors.OfType<DeprecatedNodeExecutionError>());
// or filtering by custom code
var criticalErros = result.Errors?.Where(e => e.Code != "DEPRECATED_NODE");

This is because of some two-lever errors wrapping which is made in GraphQLEngine.cs#L289.

  • So, is there any reason for such wrapping which makes it harder to determine my errors later and changes some data (ExecutionError.Code which i set in original error is overridden with class name)?
  • And if so, how can I manage such situation?
@huysentruitw
Copy link
Contributor

Coincidence or not, I've just opened this issue 30 minutes ago 🙂

@huysentruitw
Copy link
Contributor

huysentruitw commented Feb 8, 2019

While writing the unit-tests for this PR #122
I've figured that, without custom error transformation, the level of nesting is the same. So I'm not sure if it'll help anything?

@tlil
Copy link
Collaborator

tlil commented Feb 10, 2019

@BilyachenkoOY can you comment on whether @huysentruitw PR helps you or not?

@BilyachenkoOY
Copy link
Contributor Author

@tlil, @huysentruitw I checked PR and it is fine for me, since my issue was about errors added with middleware and not about thrown exceptions. And now those errors are preserved as I'd expect.
As for thrown exceptions, now I'll be able to write own transformation to unwrap such errors.

So, thank you for this PR!

tlil pushed a commit that referenced this issue Feb 10, 2019
* Allow custom error transformation on GraphQLEngine

* Make IErrorTransformation.Transform syncrhonous

* Use Enumerable.Empty instead of creating empty array
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants