A way to post-process the response about express-graphql HOT 11 CLOSED

Anybody can show an example of how to do this:

Return 400 on validation error; 500 on any other error

I wasn't sure if closing the issue meant it was addressed and if it's possible to do this now.

from express-graphql.

Still, although this hack allows me to sanitize the error messages, it doesn't give me an easy way to access the parsed query and variables from request body / query string.

Ideally I'd like to do something like:

app.use('/graphql', graphqlHTTP({
  schema: schema,
  graphiql: true,
  writeResponse: (query, variables, result, req, res) => {
    if (result.errors) {
      logger.error("Error executing query: " + query + "\nVariables:\n" + JSON.stringify(variables) + "\nErrors:\n" + JSON.stringify(result))
      res.send(500).json(sanitizeErrors(result))
    } else {
      res.send(200).json(result)
    }
  }
})

from express-graphql.

@guikubivan I just proved it out locally not sure if it is a good idea. It doesn't look bad to me but I would love feedback from anyone.

  graphqlHTTP(async (request, response) => ({
    formatError: (error: GraphQLError) => {
      response.status(400);
      ...

A more real life example would do something like:

 import { formatError, GraphQLError } from 'graphql';

  graphqlHTTP(async (request, response) => ({
    formatError: (error: GraphQLError) => {
      const { originalError } = error;
      // add custom error logic
      if(isValidationError(originalError) {
          response.status(400);
      }
      return formatError(error);
  }))

from express-graphql.

A nasty hack that I'm currently using to achieve this:

app.use('/graphql', function(req, res, next) {
  let end = res.end

  res.end = function(chunk, encoding) {
    res.end = end

    let contentType = res.get('content-type') || 'text/html'
    if (contentType.indexOf('application/json') === 0) { // don't overwrite graphiql responses
      let response = JSON.parse(chunk)

      if (response.errors) {
        let validationErrors = response.errors
          .filter(({message}) => message.indexOf('Syntax Error') !== -1 || message.indexOf('Cannot query field') !== -1)
          .map(({message}) => ({ message }))

        if (validationErrors.length > 0) {
          res.status(400).json({ errors: validationErrors })
        } else {
          res.status(500).json({ errors: [{ message: "Query could not be executed" }] })
        }
      } else {
        res.end(chunk, encoding)
      }
    } else {
      res.end(chunk, encoding)
    }
  }

  return next()
})

from express-graphql.

Those use cases sound like things we should support, but post-processing the response is not how I would prefer them implemented.

The goal of this package is to create a standardized http interface for GraphQL, which post-processing could easily circumvent.

I'll propose some paths forward:

Log errors

graphql-js could already use a comprehensive logging interface, regardless of if it is accessed via http. I'd like to focus a logging interface on graphql-js itself, and just make that easy to interface via express-graphql.

Sanitize error messages from any sensitive information

Similar to the above. Sanitizing sensitive info in errors seems like an issue regardless of the access mechanism.

Return 400 on validation error; 500 on any other error

This just seems like the right thing to do, let's standardize that.

Add custom response headers based on response body

Could you elaborate on this? What custom headers are you planning to send?

from express-graphql.

Return 400 on validation error; 500 on any other error.

Turns out this was already the current behavior, but there were some other underspecified error codes which should now be correct as of 546191d

from express-graphql.

Yes, I agree now that post processing probably isn't the best way to approach this. If logging errors and sanitizing them from responses could be baked directly into graphql-js, that would fully solve my use case.

Add custom response headers based on response body

Could you elaborate on this? What custom headers are you planning to send?

Actually, this was just something I grabbed from another issue - seemed like something the post processing could also fix.

from express-graphql.

Could you elaborate on this? What custom headers are you planning to send?

I have a use case where my GraphQL resolver hits another HTTP service, which returns certain headers for data like pagination. I'd need to forward those headers to the GraphQL client that made the initial request.

Is that currently supported without @mnylen's hack?

from express-graphql.

@vitosamson How would you know which field the pagination headers applied to?

from express-graphql.

you're right - I realized that limitation after I posted the comment. I wound up wrapping paginatable types so that they return both the list and the pagination data, and my HTTP resolvers just grab the headers from the response and add them onto the returned data. A query winds up looking like query { thingList { things {}, pagination }}, which is a bit cumbersome but flexible.

from express-graphql.

@vitosamson - you should check out https://facebook.github.io/relay/docs/graphql-connections.html where there are in depth docs on best practices for pagination.

@mnylen - I think you're right that the best solution here is to allow GraphQL.js the ability to operate on errors to do this sort of thing. Tracking that in graphql/graphql-js#284

That said, there are still some errors that occur within this package, so having the ability to sanitize at the last step is totally reasonable.

from express-graphql.