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

[FR] Improve errors tracing #1092

Open
frederikhors opened this issue Sep 29, 2022 · 1 comment
Open

[FR] Improve errors tracing #1092

frederikhors opened this issue Sep 29, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@frederikhors
Copy link
Contributor

Eliza from the tokyo's team (I don't tag here to not bother them again) just gave me an answer that might solve #1089.

Apart from the issue fixed by #1091 they suggest to use the below code to better trace errors chains:

Similarly, the reason the tracing event includes only the top-level error message is because the async-graphql authors chose to write

error = %err.message(),

when emitting the tracing event I linked above. That code is saying "record a field named error whose value is err.message(), formatted using fmt::Display.

If the error type in this code implements std::error::Error, it should instead be possible to use the tracing::Value implementation for dyn Error trait objects, in which case, the tracing subscriber would receive the value as a &dyn Error + 'static. This would allow it to record the error chain as well as the top-level error's message, which many tracing::Subscriber implementations (such as the fmt subscriber you're using) will do.

To summarize, it's possible to record errors the way you want using tracing, but the code in async-graphql isn't recording the error the way you want to. It's possible that crate's maintainers would accept a pull request and/or issue to change how the error event is recorded. I think all that's necessary should be to change the code I linked above to something like this:

                tracinglib::error!(target: "async_graphql::graphql",
                                   error = &err
                                   "error");

or, if the compiler doesn't like that, you may need to explicitly cast to dyn Error:

                tracinglib::error!(target: "async_graphql::graphql",
                                   error = &err as &(dyn std::error::Error + 'static)
                                   "error");

What do you think?

I would also like to have the option to choose whether to print the error chain. In some cases, for example, it might not help and the error alone would be fine.

@frederikhors frederikhors added the enhancement New feature or request label Sep 29, 2022
@frederikhors frederikhors changed the title [FR] Improve tracing errors [FR] Improve errors tracing Sep 29, 2022
@frederikhors
Copy link
Contributor Author

I think this can be closed after reasoning about #1094 and after #1091.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant