Apollo-server: didEncounterErrors and willSendResponse are not called on AuthenticationError

[mhart]

Using apollo-server-express 2.12.0.

According to this lifecycle, willSendResponse should always be called, and didEncounterErrors should at least be called if there's an error:

https://www.apollographql.com/docs/apollo-server/integrations/plugins/#request-lifecycle-event-flow

However, we're only seeing requestDidStart being called, and nothing more, if we have an authentication error. We're using graphql-shield 7.2.6 for our authorization. The response is returned to the client, but willSendResponse is never called (and nor is didEncounterErrors).

Eg, given the following:

const server = new ApolloServer({
  plugins: [
    {
      requestDidStart() {
        console.log('requestDidStart')
      },
      parsingDidStart() {
        console.log('parsingDidStart')
      },
      didEncounterErrors() {
        console.log('didEncounterErrors')
      },
      willSendResponse() {
        console.log('willSendResponse')
      },
    },
  ],
  // ...

All we see output in our logs is:

And we get a valid response with an errors array with message, location, stacktrace, etc all looking as they should for an authentication error.

So neither parsingDidStart nor willSendResponse is invoked – somehow these are bypassed, which indicates there's either a bug, or the lifecycle is not accurately documented.

[trickleup]

I'm seeing the same for PersistedQueryNotFound errors, only requestDidStart is called, and not didEncounterErrors or willSendResponse. apollo-server-core version 2.12.0.

[trevor-scheer]

Hey @mhart, please take a closer look at the examples in the docs. You'll notice the API for a plugin is slightly different from how you've used it in your snippet.

requestDidStart is a function that _returns an object_ of the other hooks you're looking to use. This allows plugins to maintain a local context or scope during the course of a request that the other hooks can consume.

I believe what you really want is this:

const server = new ApolloServer({
  plugins: [
    {
      requestDidStart() {
        console.log('requestDidStart');
        return {
          parsingDidStart() {
            console.log('parsingDidStart')
          },
          didEncounterErrors() {
            console.log('didEncounterErrors')
          },
          willSendResponse() {
            console.log('willSendResponse')
          },
        };
      },
    },
  ],
  // ...

@trickleup does this apply to you as well or are you experiencing the issue differently? Please provide a reproduction if you think there's a bug that needs to be addressed. Thanks!

[mhart]

Oof, I didn't see that. A little confusing as an API, but definitely my bad for not reading correctly, sorry about that!

[trickleup]

Thanks @trevor-scheer! Yes, with your example the didEncounterErrors hook does fire for the persisted query exceptions. My bad, thanks for clarifying!

[robross0606]

Hey @mhart, please take a closer look at the examples in the docs. You'll notice the API for a plugin is slightly different from how you've used it in your snippet.

requestDidStart is a function that _returns an object_ of the other hooks you're looking to use. This allows plugins to maintain a local context or scope during the course of a request that the other hooks can consume.

I believe what you really want is this:

const server = new ApolloServer({
  plugins: [
    {
      requestDidStart() {
        console.log('requestDidStart');
        return {
          parsingDidStart() {
            console.log('parsingDidStart')
          },
          didEncounterErrors() {
            console.log('didEncounterErrors')
          },
          willSendResponse() {
            console.log('willSendResponse')
          },
        };
      },
    },
  ],
  // ...

@trickleup does this apply to you as well or are you experiencing the issue differently? Please provide a reproduction if you think there's a bug that needs to be addressed. Thanks!

When I tried to follow this example I got an error:

TypeError: endHandler is not a function
    at C:\Code\app\node_modules\graphql-extensions\dist\index.js:80:21
    at Object.<anonymous> (C:\Code\app\node_modules\apollo-server-core\dist\requestPipeline.js:201:13)
    at Generator.next (<anonymous>)
    at fulfilled (C:\Code\app\node_modules\apollo-server-core\dist\requestPipeline.js:5:58)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

The syntax of this API is very confusing. What exactly is supposed to be returned for the handlers from requestDidStart()?

[robross0606]

The syntax of this API is very confusing. What exactly is supposed to be returned for the handlers from requestDidStart()?

If this is supposed to return functions, shouldn't it look more like this?

        return {
          parsingDidStart: () => {
            console.log('parsingDidStart')
          },
          didEncounterErrors: () => {
            console.log('didEncounterErrors')
          },
          willSendResponse: () => {
            console.log('willSendResponse')
          },
        };

[mhart]

@robross0606 That's the same thing. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Method_definitions

[robross0606]

yeah, I realized that after I wrote it. Then what is causing the errors?

[andreme]

@trevor-scheer The docs (now) do not show that some handlers need to be returned from the requestDidStart handler.
For example willSendResponse looks like it should just be a function on the plugin itself.
https://www.apollographql.com/docs/apollo-server/integrations/plugins/#willsendresponse

[alexbjorlig]

looks like it should just be a function on the plugin itself

What does this mean? I am trying to write a plugin called willSendRespond and no matter how I try to write it, it's never called?

Update, it now works 🥳 :

plugins: [
    {
        requestDidStart(requestContext) {
            /* Within this returned object, define functions that respond
                to request-specific lifecycle events. */
            return {
                /* The `parsingDidStart` request lifecycle event fires
                when parsing begins. The event is scoped within an
                associated `requestDidStart` server lifecycle event. */
                willSendResponse(requestContext) {
                    console.log('willSendResponse');
                },
            };
        },
    },
],