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 the context parameter to the didEncounterErrors handler #3157

Closed
wants to merge 2 commits into from

Conversation

cuchi
Copy link

@cuchi cuchi commented Aug 14, 2019

This change enables us to use the current Context when handling errors via the didEncounterErrors handler.

Consider this scenario, where the Context is relevant to send the user information to an error reporting tool:

class MyErrorHandler extends GraphQLExtension<Context> {

    didEncounterErrors(errors: ReadonlyArray<GraphQLError>, context: Context) {
        for (const error of errors) {
            if (isUnexpected(error)) {
                reportError(error, context.user)
            }
            // ...
        }
    }

}

Fixes #2702

@apollo-cla
Copy link

@cuchi: 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/

@abernix
Copy link
Member

abernix commented Aug 15, 2019

We're not continuing to develop the graphql-extensions API and we're intending on deprecating it in Apollo Server 3.x with the introduction of the new request pipeline in #1795, which supersedes the previously undocumented graphql-extensions and untested API which was only intended for internal use.

This new request pipeline API (which I might add is strongly typed, unlike its predecessor) already supports this via its (same purpose) didEncounterErrors hook, which receives the context directly (and with a consistent positional parameter pattern, like its sibling hooks):

didEncounterErrors?(
requestContext: WithRequired<
GraphQLRequestContext<TContext>,
'metrics' | 'source' | 'errors'
>,
): ValueOrPromise<void>;

If there are barriers to you adopting the new request pipeline, please do let us know if you run into any barriers to adopting it! And lastly, I appreciate you taking the time to make a contribution, but opening this PR against a closed issue does run the risk of it not being accepted. I do hope you'll find this solution better, as we think that the new request pipeline API will be a better API in the long-run.

@abernix abernix closed this Aug 15, 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.

GraphQLExtension.didEncounterErrors with context or requestContext
3 participants