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

Add errors details to didEncounterErrors hook #6558

Conversation

b1zzu
Copy link

@b1zzu b1zzu commented Jun 10, 2022

In case an error is a SyntaxError or ValidationError this information is not passed to the didEncounterErrors hook. This missing piece of information is very important for logging plugins that may use it as a filter key to creating better metrics and monitoring alerts.

@apollo-cla
Copy link

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

@netlify
Copy link

netlify bot commented Jun 10, 2022

Deploy Preview for apollo-server-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit da95b68
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/62a307ab7b25ea00084f81db

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit da95b68:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@glasser
Copy link
Member

glasser commented Jun 13, 2022

This seems like it would be a backwards-incompatible change so it probably isn't appropriate for AS3.

We've reworked our error handling code a lot on the version-4 branch. Among other things we don't actually end up using SyntaxError and ValidationError any more. It does look like we're still just passing the raw errors to didEncounterErrors, though. I am curious — is it possible that formatErrors would be a better place than didEncounterErrors to do your logging? Alternatively, perhaps we should apply something like this PR to version-4 where this backwards-incompatible change would be allowed.

@IvanGoncharov any thoughts?

@glasser glasser closed this Jun 13, 2022
@glasser
Copy link
Member

glasser commented Jun 13, 2022

(Or something like passing didEncounterErrors two arguments: the unformatted and formatted errors? (Or maybe the second argument would be an options object containing the formatted errors and maybe more stuff later))

IvanGoncharov added a commit to IvanGoncharov/apollo-server that referenced this pull request Jun 14, 2022
Implement fix proposed in apollographql#6558 but implement it on top of AS4
Also contain some cleanup and refactoring related to apollographql#6355
IvanGoncharov added a commit to IvanGoncharov/apollo-server that referenced this pull request Jun 14, 2022
Implement fix proposed in apollographql#6558 but implement it on top of AS4
Also contain some cleanup and refactoring related to apollographql#6355
IvanGoncharov added a commit to IvanGoncharov/apollo-server that referenced this pull request Jun 17, 2022
Implement fix proposed in apollographql#6558 but implement it on top of AS4
Also contain some cleanup and refactoring related to apollographql#6355
IvanGoncharov added a commit to IvanGoncharov/apollo-server that referenced this pull request Jun 30, 2022
Implement fix proposed in apollographql#6558 but implement it on top of AS4
Also contain some cleanup and refactoring related to apollographql#6355
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 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.

None yet

3 participants