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

Fastify Session - Does not set cookies when using res.raw #56

Closed
2 tasks done
gino opened this issue Jan 10, 2022 · 4 comments
Closed
2 tasks done

Fastify Session - Does not set cookies when using res.raw #56

gino opened this issue Jan 10, 2022 · 4 comments

Comments

@gino
Copy link

gino commented Jan 10, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the issue has not already been raised

Issue

Hi there,

I am using Fastify in combination with @fastify/session (with the required cookie plugin aswell) and graphql-helix with this fastify example. In this example, you can see that on line 37 res.raw is being used. However, whenever I use this in combination with req.session, the cookie does not get set whenever I am in the /graphql endpoint.

But whenever I use req.session in a regular endpoint, it does work. For example:

// The cookie does get set
app.get("/", (req, res) => {
  if (!req.session.test) {
    req.session.test = "some testing value";
  }

  res.send({ session: req.session.test });
});

So I think this has something to do with the res.raw or something similar to graphql-helix. I am not quite sure if this is a bug or if this has something to do with graphql-helix.

I hope someone is being able to help me out here. Thanks!

Edit: I assume that this could be expected behaviour since I am using res.raw. However, passing res instead of res.raw results in a few errors which makes sense and explains why res.raw is being used instead.

@mcollina
Copy link
Member

This module is not compatible with using something like graphql-helix as it bypass fastify by using res.raw. I would recommend you to use https://mercurius.dev/#/ instead

@n1ru4l
Copy link

n1ru4l commented Jan 11, 2022

graphql-helix does not necessarily require using res.raw. As fastify is able to accept a Readable stream for reply.send, you can cast the helix execution result from an async iterable to a Readable stream.

We are currently adding these helpers over here: contra/graphql-helix#228

The full setup look like this:

import fastify from "fastify";
import { getGraphQLParameters, processRequest, renderGraphiQL, shouldRenderGraphiQL } from "graphql-helix";
import { toResponsePayload } from "graphql-helix/to-response-payload";
import { toReadable } from "graphql-helix/node/to-readable";
import { schema } from "./schema";
const app = fastify();
app.route({
  method: ["GET", "POST"],
  url: "/graphql",
  async handler(req, reply) {
    const request = {
      body: req.body,
      headers: req.headers,
      method: req.method,
      query: req.query,
    };
    if (shouldRenderGraphiQL(request)) {
      reply.type("text/html");
      reply.send(renderGraphiQL({}));
    } else {
      const request = {
        body: req.body,
        headers: req.headers,
        method: req.method,
        query: req.query,
      };
      const { operationName, query, variables } = getGraphQLParameters(request);
      const result = await processRequest({
        operationName,
        query,
        variables,
        request,
        schema,
      });
      const responsePayload = toResponsePayload(result);
      reply.status(responsePayload.status);
      reply.headers(responsePayload.headers);
      reply.send(toReadable(responsePayload.source));
    }
  },
});
const port = process.env.PORT || 4000;
app.listen(port, () => {
  console.log(`GraphQL server is running on port ${port}.`);
});

You can use any fastify plugin with it 9https://github.com/contrawork/graphql-helix/blob/657488c43dda17eddb6713a9acb1a168b4265330/packages/core/test/integration.test.ts#L667-L704)

In the future it would definitely be great if reply.send would also accept an async iterable in addition to streams.

@mcollina
Copy link
Member

I don't think you need special wrappers to move from an async iterator to a stream, you can just do Readable.from() and it will do the trick.

@n1ru4l
Copy link

n1ru4l commented Jan 11, 2022

@mcollina Thank you for the tip, I will test it!

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

3 participants