requestDidStart hook not called if context creation failes

[ankit-m]

If an error is thrown during context creation the requestDidStart hook is not called. As a result, using didEncounterErrors is not possible for those errors.

Note: formatError function is called

[abernix]

Thanks for reporting this issue! It's possible this has been resolved, but I can't be certain because there's no reproduction here to try quickly.

As requested in the issue template when you submitted this issue, GitHub Issues need to include simple, runnable reproductions. This means either a GitHub repository that anyone can clone and run and see the problem or an example built using the Apollo Server CodeSandbox template or by "re-mixing" the Apollo Server Glitch template.

Building reproductions takes a large portion of core contributor's and issue triager's time and without a runnable reproduction, we'll have to close this issue in the next 7 days. Hope you understand!

If you aren't able to supply a runnable reproduction, we're happy to re-open this later on when one can be provided. Regardless, thanks again for filing the issue since, reproduction or not, this information could be useful for a future visitor!

[cbdt]

I have the same issue where the requestDidStart hook is not called, here's the reproduction I made. It cause the code you provide here to not work because the plugin is never called. Hope it will help!

[victorcerqueira]

+1 I'm having the exact same issue as @cbdt. Please just let us know if we're doing something wrong or if the hook is not working as intended.

[axelvaindal]

I'm having the same issue as well.
Throwing any error during context creation make the request status to be HTTP 400 with correct error in the errors array but skip the requestDidStart hook.
Although I assume this is expected (the 400 status), the hook not being called prevent us to use the workaround for the 401 http status.

[vytautas-pranskunas-]

any progress on this?

[vytautas-pranskunas-]

cannot believe it is still not done...

[glasser]

Fixing this is a challenge because of the way the codebase is currently structured. Apollo Server 1 was built as a set of standalone functions; the ApolloServer class was only introduced in Apollo Server 2. The structure of the code is that the ApolloServer class takes its internal state and produces a pretty complex object that it passes to the standalone runHttpQuery function, which then calls the processGraphQLRequest function.

Calling the context function happens via some weird machinery coordinated by the ApolloServer and runHttpQuery layers, but handling of request plugins only happens in the processGraphQLRequest layer. Fixing this would be a lot easier if we did the refactoring to merge these three layers (ApolloServer, runHttpQuery, processGraphQLRequest) into one more coherent layer.

As a work-around, you can ensure that your context function does not throw, with something like:

new ApolloServer({
  context: async () => {
    try {
      // ... actual context function here
    } catch (contextError) {
      return { contextError };
    }
  },
  plugins: [{
    async requestDidStart(requestContext) {
      if (requestContext.context.contextError) {
        throw requestContext.context.contextError;
      }
    }
  }],
});

[vytautas-pranskunas-]

This is not so good because if i rethrow in requestDidStart status code is 500 not 200

[glasser]

Even if you throw an HttpQueryError?

[vytautas-pranskunas-]

No I am rethrowing original error that was caought in context - AuthenticationError

 context: async ({ req, connection }: { req: any; connection: any }) => {
            try {
                const { isAuthorized, ...userData } = await authService.isAuthorized(req);
                if (!isAuthorized) {
                    throw new AuthenticationError('Authentication token is invalid, please log in');
                }
                return {
                 
                } as RequestContext;
            } catch (contextError) {
                return { contextError }
            }
        },

....

 requestDidStart(requestContext) {
              if (requestContext.context.contextError) {
                throw requestContext.context.contextError;
              }
            }

[glasser]

If you care about the HTTP status code, you should throw HttpQueryError.

[vytautas-pranskunas-]

Nop. I tried HttpQueryError but ststaus code was 500

 if (requestContext.context.contextError) {
                  throw new HttpQueryError(200, 'UNAUTHENTICATED', true);
              }

{
    "errors": [
        {
            "message": "UNAUTHENTICATED",
            "extensions": {
                "code": "INTERNAL_SERVER_ERROR",
                "exception": {
                    "stacktrace": [
                        "HttpQueryError: UNAUTHENTICATED",

[glasser]

Works for me, with something like

const { ApolloServer } = require('apollo-server');
const { HttpQueryError } = require('apollo-server-core');

new ApolloServer({
  typeDefs: 'type Query{x:ID}',
  plugins: [
    {
      async requestDidStart() {
        throw new HttpQueryError(411, 'bla');
      },
    },
  ],
}).listen(4231);

Happy to look into this further if you can show a complete reproduction (eg, on codesandbox.io or with git clone).

[vytautas-pranskunas-]

@glasser

intersting because for me it is

{
    "errors": [
        {
            "message": "bla",
            "extensions": {
                "code": "INTERNAL_SERVER_ERROR",
                "exception": {
                    "stacktrace": [
                        "HttpQueryError: bla",

but I am using apollo-server-express so maybe this is the reason

[glasser]

Happy to look into it more with a full reproduction.

[glasser]

In Apollo Server 4 we still don't call requestDidStart if context creation fails; that's because the GraphQLRequestContext argument to the plugin already has the contextValue on it. However, there's a new contextCreationDidFail hook to handle this case. You can throw a GraphQLError here (which can set HTTP status code and headers) if you like.

[iambumblehead]

The linked contextCreationDidFail example is inadequate, because it only shows a call to console.log rather than something practical like, for example, customizing the error response with error codes or i18n messaging.

It would be nice if there were a single, predictable, stable, normal-looking, not-buggy and not-changing-with-each-new-version hook mechanism with a practical, not-wordy, not-meandering and complete example