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

Does not support healthcheck endpoints such as /graphql/health #958

Closed
Tracked by #1358
dthyresson opened this issue Mar 21, 2022 · 9 comments
Closed
Tracked by #1358

Does not support healthcheck endpoints such as /graphql/health #958

dthyresson opened this issue Mar 21, 2022 · 9 comments
Assignees
Milestone

Comments

@dthyresson
Copy link

In RedwoodJS, when using helix, we had a healthcheck at the endpoint /graphql/health because graphql was the serverless function for the graphQL server.

const HEALTH_CHECK_PATH = '/health'

 export type OnHealthcheckFn = (event: APIGatewayProxyEvent) => Promise<any>

 export function createHealthcheckContext(
   onHealthcheckFn?: OnHealthcheckFn,
   corsContext?: CorsContext
 ) {
   return {
     isHealthcheckRequest(requestPath: string) {
       return requestPath.endsWith(HEALTH_CHECK_PATH) / / 👈
     },

Bit, with Yoga, please consider this code in

if (urlObj.pathname === '/health') {

const urlObj = new URL(request.url)
      if (urlObj.pathname === '/health') { / / 👈
        return new Response(`{ "message": "alive" }`, {
          status: 200,
          headers: {
            'Content-Type': 'application/json',
            'x-yoga-id': this.id,
          },
        })
      }
      if (urlObj.pathname === '/readiness') { / / 👈
        urlObj.pathname = '/health' / / 👈
        const readinessResponse = await fetch(urlObj.toString())
        if (
          readinessResponse.status === 200 &&
          readinessResponse.headers.get('x-yoga-id') === this.id
        ) {
          return new Response(`{ "message": "ready" }`, {
            status: 200,
            headers: {
              'Content-Type': 'application/json',
            },
          })
        }
        throw new Error(
          `Readiness check failed with status ${readinessResponse.status}`,
        )
      }

Note that with helix, we checked endsWith vs Yoga checks === the path.

  1. Could Yoga also support endsWith so a serverless function can be used?
  2. Are custom checks supported? If not, redwood has to update its docs. See: https://redwoodjs.com/docs/graphql#health-checks
  3. How does readiness different from health and when should each be used?
@dthyresson
Copy link
Author

dthyresson commented Mar 21, 2022

Note: Redwood would very much like a custom healthcheck function so we can test deploy CI and make sure a query can fetch data from the associated database. We'd add some query string to /graphql/health?some=thing to run test(s) and expect a result that we can confirm easily using Pingdom to check that it's up.

Kingdom cannot issue POSTs apparently to test -- and for some reason Netlify may be encoding or double encoding

https://rwjs-deploy-target-ci.netlify.app/.netlify/functions/graphql?query=%7Bredwood%7Bversion%7D%7D

As the response is

{"errors":[{"message":"Must provide query string."}]}

cc @thedavidprice

@thedavidprice
Copy link

Previously (and on "most" hosting providers) I was able to do something like this:
https://deploy-target-ci.vercel.app/api/graphql?query={post(id:1){title}}

Which made for an easy health check:

  • graphql api ✅
  • DB queries ✅

@ardatan
Copy link
Collaborator

ardatan commented Mar 21, 2022

GET requests are handled by Yoga;
#959
I think event.path (we used to create the Request object) doesn't have search params.

About health check and readiness endpoints;
#960

I think it is better them to configure them instead of endsWith because endsWith looks a bit unsafe.

@thedavidprice
Copy link

GET requests are handled by Yoga

Ok, then there must be something going on elsewhere. I can confirm for some hosting providers GET is working:

But not for others:

@ardatan
Copy link
Collaborator

ardatan commented Mar 22, 2022

I think I changed my mind and now I agree with you on using endsWith. Now Yoga checks the path in this way. Let me know if that works for you, then we can close this issue.
@dthyresson

@dthyresson
Copy link
Author

I think I changed my mind and now I agree with you on using endsWith. Now Yoga checks the path in this way. Let me know if that works for you, then we can close this issue.
@dthyresson

It does - one question: will Yoga support custom health check functions in the future?

@ardatan
Copy link
Collaborator

ardatan commented Mar 22, 2022

I'm working on that @dthyresson
How would you imagine that?

@dthyresson
Copy link
Author

dthyresson commented Mar 22, 2022

I'm working on that @dthyresson
How would you imagine that?

Ideally, it would work the same as documented here https://redwoodjs.com/docs/graphql#health-checks

This is how RedwoodJS declared one with Helix -- and keeping the same method means we wouldn't have to change docs or tell people for to do differently.

// api/src/functions/graphql.{ts,js}

const myCustomHealthCheck = async () => {
  if (ok) {
    // Implement your custom check, such as:
    // * invoke an api
    // * call a service
    // * make a db request
    // that ensures your GraphQL endpoint is healthy
    return
  }

  throw Error('Health check failed')
}

export const handler = createGraphQLHandler({
  onHealthCheck = await myCustomHealthCheck(),
  // .. other config
  getCurrentUser,
  directives,
  sdls,
  services,
})

And then that function was used before like:

  const healthcheckContext = createHealthcheckContext(
    onHealthCheck,
    corsContext
  )

Where the createHealthcheckContext was:

import type { APIGatewayProxyEvent } from 'aws-lambda'
import { Request } from 'graphql-helix'

import { CorsContext } from './cors'

const HEALTH_CHECK_PATH = '/health'

export type OnHealthcheckFn = (event: APIGatewayProxyEvent) => Promise<any>

export function createHealthcheckContext(
  onHealthcheckFn?: OnHealthcheckFn,
  corsContext?: CorsContext
) {
  return {
    isHealthcheckRequest(requestPath: string) {
      return requestPath.endsWith(HEALTH_CHECK_PATH)
    },
    async handleHealthCheck(request: Request, event: APIGatewayProxyEvent) {
      const corsHeaders = corsContext
        ? corsContext.getRequestHeaders(request)
        : {}

      if (onHealthcheckFn) {
        try {
          await onHealthcheckFn(event)
        } catch (_) {
          return {
            body: JSON.stringify({ status: 'fail' }),
            statusCode: 503,
            headers: {
              'Content-Type': 'application/json',
              ...corsHeaders,
            },
          }
        }
      }

      return {
        body: JSON.stringify({ status: 'pass' }),
        statusCode: 200,
        headers: {
          'Content-Type': 'application/json',
          ...corsHeaders,
        },
      }
    },
  }
}

@klippx
Copy link
Contributor

klippx commented Mar 28, 2022

We are also using healthchecks but we are using them on separate endpoints that are not Yoga related. To add to the context we use express app and only mount yoga on relevant endpoints. I have seen the readiness checks in the source code and have thought to myself that it looks a bit odd to me, but it's not my use case, but it's hard coded strings and not very configurable.

But since it is part of Yoga's ambition to cater for healthchecks, then I would agree they should be more customizable. Perhaps

import { createServer } from '@graphql-yoga/node'

const yogaServer = createServer({
  // default: `false`, no healthcheck added by default
  healthcheck: {
    // No default, how would you know what the user wants?
    endpoint: '/readiness',
    // async fn that resolves or rejects, default `async () => ({ "message": "ready" })`. If it resolves 200 OK is returned with the resolved value.
    check: async () => { await checkDBConnection() },
  },
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants