onHealthCheck is never called

[arizonatribe]

This issue has been brought up before but was prematurely closed without actually testing the solution.

Essentially the Apollo Server implementations for Express, Hapi, Koa, etc. each need to be altered to properly honor a custom onHealthCheck handler passed in by the user. This should be if it is passed into the ApolloServer constructor or if it is passed into the .applyMiddleware() method.

Currently the code (using Apollo Server Express as an example) essentially does this:

do you have onHealthCheck?
---> invoke it
---> do nothing with your return value
---> always return {status: 'pass'}
---> unless your handler errored, then {status: 'fail'}

What it probably should be doing is:

do you have onHealthCheck?
---> invoke it
---> capture any data returned by your handler
---> add your data to the default return object of {status: 'pass'}
---> unless your handler errored, then {status: 'fail'}

---

Essentially what happens right now is a custom health check handler is completely ignored if it is passed in via the Apollo Server constructor. However it does get invoked if passed in via the .applyMiddleware(). That method invokes this.getMiddleware() which registers the route for the custom health check.

So in addition to processing the return object from a custom health check handler, the constructor for the extended ApolloServer (which is extended in each of the libraries for Hapi, Express, Koa, etc.) is to do something with the custom handler. My guess is you could just place it onto the class instance itself, as it doesn't quite "feel" right to register a route there and risk double-defining it later.

So, for the Express implementation you might do something like this in the constructor:

        super(config);
        if (config.onHealthCheck) {
          this.onHealthCheck = config.onHealthCheck
        } else {
          this.onHealthCheck = (req, res) => {
            res.json({ status: 'pass' });
          }
        }

and then inside the getMiddleware() method it would look mostly the same as now, but with a few alterations:

if (!disableHealthCheck) {
  router.use('/.well-known/apollo/server-health', async (req, res) => {
    res.type('application/health+json');
    try {
      const result = await (onHealthCheck || this.onHealthCheck)(req, res)
      if (!res.headersSent) {
        res.json({ status: 'pass', ...(result || {}) })
      }
    } catch (err) {
      res.status(503).json({ status: 'fail' });
    }
  });
}

That way you give consumers the option to do whatever they want with the connect middleware request/response objects, or to send back data for you to merge on top of the default response.

[glasser]

The health check feature was removed from Apollo Server 4. You're welcome to define arbitrary HTTP handlers with arbitrary behavior using your web framework; you don't need your GraphQL operation execution middleware to do that for you.