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

Use Task.yield/2 instead of Task.await/2 in batching fn, so we can log error #1298

Merged

Conversation

VladoPlavsic
Copy link
Contributor

  • Improve absinthe batch logging, add useful information on what module what function with what parameters is failing when timeout occurs

  • Instead of calling Task.await/2 which kills process after timeout has expired, use Task.yield/2, if it timeouts, kill task and log which function failed to resolve, after which we continue to act like Task.await/2 and kill calling process (self).

…, what function with what parameters is failing when timeout occures

Instead of calling Task.await/2 which kills process after timeout has expired, use Task.yield/2, if it timeouts, kill task and log which function failed to resolve, after which we continue to act like Task.await and kill calling process (self).
Copy link
Contributor

@benwilson512 benwilson512 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great change, thanks!

@benwilson512 benwilson512 merged commit 2fd4634 into absinthe-graphql:main Feb 14, 2024
@rossvz
Copy link

rossvz commented Jun 26, 2024

@VladoPlavsic @benwilson512 I ended up trying to reimplement this today and noticed its merged to main, but not in latest -v1.7.6. Is there any chance of this making it into a release soon? This would help us a ton with tracking down failed batch resolvers in our API!

@katafrakt
Copy link
Contributor

By the way, I wonder if it's good for a library to just log things like this. What if someone has different logging format or needs or even some security considerations of logging the whole fun? Wouldn't it be better to execute a telemetry event instead and everyone could subscribe to it and log as they want?

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

Successfully merging this pull request may close these issues.

4 participants