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

enable async in formatError hook #1748

Closed
wants to merge 1 commit into from
Closed

enable async in formatError hook #1748

wants to merge 1 commit into from

Conversation

anymost
Copy link

@anymost anymost commented Sep 29, 2018

@apollo-cla
Copy link

@anymost: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@martijnwalraven
Copy link
Contributor

Can you explain in more detail why you need the ability for formatError to run asynchronously? As this function is in the critical path for generating a response, it should return as fast as possible. And I have trouble thinking of use cases that would require async behavior for formatting.

@anymost
Copy link
Author

anymost commented Oct 4, 2018

@martijnwalraven Hi,The reason why I use async/await in formatError is we have so internal tools. The tools are used to report error and so on, But most tools are run async asynchronously, so I want to formatError support asynchronous logic.

@anymost
Copy link
Author

anymost commented Oct 4, 2018

meantime, formatResponse hook support async/await, I take formatError hook supporting async/await for granted

@eltonio450
Copy link

@martijnwalraven we also use async function to log errors (via Sentry), and would like to use the formatError hook to do so.

Do you think this PR is going to be merged in master in the future ?

thank you !!

@eltonio450
Copy link

(to be even more specific, we use the serverless version of apollo server, and everything needs to be handled before the return. For now, we receive empty errors + erros are not reported in sentry

@abernix
Copy link
Member

abernix commented May 27, 2019

Thanks for opening this originally.

As of #2719 (which is included in the latest alpha releases of Apollo Server 2.6.0 on the next npm dist-tag; tracked in #2669) we now include a didEncounterErrors life-cycle hook as part of the request pipeline events. It supports async so should hopefully be exactly what you need in order to implement this and cover this use-case. Please let us know if this doesn't tick that box!

@abernix abernix closed this May 27, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants