Propagate HTTP status codes about graphql-weaver HOT 14 CLOSED

I assume your nginx reverse proxy is used as one endpoint in a weaver config. In grpahql-weaver, the GraphQLClient instance assigned to an endpoint config (client property) is responsible of handling all the HTTP stuff. If you specify the url property, the default HttpGraphQLClient implementation will be used. If the status code is not 200, it determines whether the response is still a valid GraphQL response (a JSON object with an errors property). If that's the case, it is handled as if the status was 200 with these error messages. If not, it throws a generic exception with the status code.

graphql-weaver currently fails a whole query as soon as one requested endpoint reported an error. This is something we intend to improve in the near future, so that the successful endpoints are still included successfully in the result. 1)

A GraphQLSchema can only report GraphQL errors - the same applies for a woven schema. Which HTTP status code you set is determined by the server implementation. If you can configure/contorl the server implementation, it's up to you to send whichever exit code you like.

Did this information help?

Edit: 1) I just implemented proper error handling (not released yet, need to perform some tests first). Errors are reported as local as possible and error locations are properly mapped.

from graphql-weaver.

@gengjiawen It looks like you deleted your comment, did you solve the problem already? This is was I was going to reply:

There is no straightforward answer because firstly this depends on the graphql server implementation you use and secondly you need to decide how to combine the different HTTP response statuses different endpoints reported. But as a guideline, I would suggest that you change or configure your graphlql server implementation (I don't know which one you use) so that it

• looks into the errors part of the response and filters the error messages for a regex /GraphQL endpoint at ([^\s]+) reported ([0-9]+) .+/
• settles on one status code using the matches (in case there are multiple errors with status codes)
• uses this status code

This is a bit brittle because it relies on string matching. To improve this, you can extend HttpGraphQLClient and throw a typed error with a serializable property (e.g. statusCode) that you can use to filter on instead. This would also be a welcome pull request.

from graphql-weaver.

No, I have not solve it. I deleted it because I have not figure out a few questions. Thanks for the replay, I will give it a try. For now I run into a problem, the status code from Query is 500 and Mutation 200 if I pass the wrong token.
query request:

query {
  viewer {
    id
  }
}

query res (status code is 500):

{
  "errors": [
    {
      "message": "GraphQL error: You are not authorized to perform that action.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "viewer"
      ]
    }
  ],
  "data": null
}

Is graphql-express causing this, since data is null ?

from graphql-weaver.

Is graphql-express causing this, since data is null ?

Looks like it. That's a bit strange because null data is just a special case if all fields in the error path are NonNull. I guess if you make viewer nullable, data would no longer be null (but {viewer: null}) and thus the response should be 200, as it is for mutations.

apollo-server has an option formatResponse which should be what you are looking for. Another approach would be to add some middleware after graphql-express, see some of these suggestions.

from graphql-weaver.

Thanks. Actually, my first thought to fix the status code is to write a express middleware to hijack the status code. But for now, I have not got any progress, mainly I don't want 500 as a status code using graphql-express. I will try to switch to apollo-server to see can they have a way to fix the forward header.

from graphql-weaver.

By the way, the 502 problem seems like a issue I caused, because I use setInterval to check schema update, when I have found new shema, I use nodemon to restart my server. I deleted those code, and the problem goes away. Not quite sure what causing this.

from graphql-weaver.

Glad you're making progress. Just a side note: You don't need to restart the whole node process when the schema changes. You can just call weaveSchemas again and store the new schema, like this:

async function startServer() {
  let currentSchema = await createSchema();

  const app = express();

  app.use('/graphql', bodyParser.json(), 
    graphqlExpress(req => {
      return ({
        schema: currentSchema ,
        context: req
      });
    })
  );

  app.listen(PORT);

  // or something fancier to detect changes
  setInterval(async function() {
    currentSchema = await createSchema();
  }, 10000);
}

async function createSchema() {
  return weaveSchemas(/* ... */)
}

from graphql-weaver.

The refresh solution you provided is valid, thanks. I also have some progress on handle status code.

from graphql-weaver.

In handle the status code, I run into some issue. In order to return the status code I want, I did something like this

  const app = new koa();
  const router = new koaRouter();
  app.use(async (ctx, next) => {
    try {
      await next();
    } catch (err) {
      ctx.response.status = err.statusCode || err.status || 500;
      ctx.response.body = {
        message: err.message
      };
    }
  });

  // koaBody is needed just for POST.
  router.post('/graphql', koaBody(),
    (ctx, next) => {
      console.log("m1 start");
      console.log(ctx.request);
      next();
      console.log(ctx.response);
      console.log(ctx.body);
      console.log("m1 end");
    }
    ,graphqlKoa(req => {
      return ({
        schema: currentSchema,
        context: req
      });
    }));

  router.get('/graphiql', graphiqlKoa({endpointURL: '/graphql'}));

But I can't catch the below error. Any thing I can do to catch the error ?

TraceError: GraphQL error: You are not authorized to perform that action.
    at TraceError.Exception (/graphql-gateway/node_modules/trace-error/dist/Exception.js:82:17)
    at new TraceError (/graphql-gateway/node_modules/trace-error/dist/TraceError.js:173:84)
    at Object.assertSuccessfulResult (/graphql-gateway/node_modules/graphql-weaver/src/graphql/execution-result.ts:9:19)
    at ResolverTransformer.<anonymous> (/graphql-gateway/node_modules/graphql-weaver/src/pipeline/proxy-resolvers.ts:58:30)
    at step (/graphql-gateway/node_modules/graphql-weaver/dist/src/pipeline/proxy-resolvers.js:40:23)
    at Object.next (/graphql-gateway/node_modules/graphql-weaver/dist/src/pipeline/proxy-resolvers.js:21:53)
    at fulfilled (/graphql-gateway/node_modules/graphql-weaver/dist/src/pipeline/proxy-resolvers.js:12:58)
    at <anonymous>
    at process._tickDomainCallback (internal/process/next_tick.js:228:7)
    {
      "message": "You are not authorized to perform that action.",
      "status": "401"
    }

from graphql-weaver.

Ah, I guess the endpoint still serves a valid GraphQL response where errors is populated. In this case, you won't even get the status code in the error message because we just pass through the errors. In the current version, you get an ugly TraceError, but in the upcoming version 0.11, the response will transparently include the errors at their right positions.

For your use case, this means that you probably don't even want to throw an error within the GraphQLClient. Instead, I'd suggest to collect the status codes in the GraphQL context. The context is determined by the graphql server and it gets passed around to all clients. In your case it's the request.

You could try something like this:

class CustomHttpGraphQLClient extends GraphQLClient {
    protected async fetchResponse(document: DocumentNode, variables?: { [name: string]: any }, context?: any, introspect?: boolean): Promise<Response> {
        const response = await super.fetchResponse(document, variables, context, introspect);
        if (!response.ok) {
            context.endpointStatusCode = res.status;
        }
        return response;
    }
}

const currentSchema = weaveSchemas({
    endpoints: [{
        client: new CustomHttpGraphQLClient('url...')
    }]
});

  const app = new koa();
  const router = new koaRouter();
  app.use(async (ctx, next) => {
    await next(); // graphql execution
    if (ctx.request.endpointStatusCode) {
      ctx.response.status = ctx.request.endpointStatusCode;
    }
  });

  // koaBody is needed just for POST.
  router.post('/graphql', koaBody(),
    (ctx, next) => {
      console.log("m1 start");
      console.log(ctx.request);
      next();
      console.log(ctx.response);
      console.log(ctx.body);
      console.log("m1 end");
    }
    ,graphqlKoa(req => {
      return ({
        schema: currentSchema,
        context: req
      });
    }));

  router.get('/graphiql', graphiqlKoa({endpointURL: '/graphql'}));

from graphql-weaver.

I run into this error, index.ts (53,21): Property 'endpointStatusCode' does not exist on type 'Request'. Looks like need any way to pass.
The context in koa defines like this :

    interface Context extends BaseContext {
        app: Application;
        request: Request;
        response: Response;
        req: IncomingMessage;
        res: ServerResponse;
        originalUrl: string;
        cookies: Cookies;
        accept: accepts.Accepts;
        state: any;
        /**
         * To bypass Koa's built-in response handling, you may explicitly set `ctx.respond = false;`
         */
        respond?: boolean;
    }

from graphql-weaver.

ha, I have an idea. The Context state looks something I can hook into.

from graphql-weaver.

Solved by something like this:

  protected async fetchResponse(document: DocumentNode, variables?: { [name: string]: any }, context?: any, introspect?: boolean): Promise<Response> {
    const response = await super.fetchResponse(document, variables, context, introspect);
    if (!response.ok) {
      context.state = {raw: response};
    }
    return response;
  }

  app.use(async (ctx, next) => {
    console.log('me start');
    await next(); // graphql execution
    if (ctx.state.raw !== undefined) {
      const raw: Response = ctx.state.raw;
      const body = await raw.text();
      console.log(`status: ${raw.status}, body: ${body}`);
      ctx.status = raw.status;
      ctx.body = body;
    }
    console.log('me end');
  });

from graphql-weaver.

Thanks for your great patience and help. I believe all my problem has been resolved :)

from graphql-weaver.