fix(fastify): return reply object from async handlers

[SimenB]

I'm trying to use this module with fastify@4, but we never got a response. Adding the return statements fixed that issue. It's somewhat mentioned in the migration guide (https://medium.com/@fastifyjs/fastify-v4-ga-59f2103b5f0e).

Note that this also works in fastify 3

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5f72a7e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[netlify]

✅ Deploy Preview for apollo-server-docs canceled.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	ef350dc
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-server-docs/deploys/6308016e746ea000083c240e

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ef350dc:

Sandbox	Source
Apollo Server Typescript	Configuration
Apollo Server	Configuration

[SimenB]

Not sure how to add a changeset, doesn't seem set up?

$ npx changeset
npm ERR! could not determine executable to run

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/simen/.npm/_logs/2022-08-11T11_38_58_808Z-debug-0.log

[glasser]

We've come to the conclusion that supporting Fastify v3 and v4 in the same release of apollo-server-fastify is not feasible (#6576 (comment) and some other comments). The forthcoming AS4 disentangles the precise versions of web framework integrations from Apollo Server's own version and should make supporting any Fastify version easy. (It will also allow us to hand off maintenance of the framework integration to people who actually use Fastify in any context other than trying to evaluate PRs like this one.)

(Also, we're not using changeset yet on main/Apollo Server 3. I should see if we can turn off the changeset bot for now, sorry.)

Is this PR also helpful for Fastify v3 users?

[SimenB]

Is this PR also helpful for Fastify v3 users?

Yes, making sure to return Reply from handlers have fixed bugs in our own (currently v3) code base, albeit not when using .send. Support was added back in https://github.com/fastify/fastify/pull/1869/files#diff-e5866ff20d3d7c44302e5bf2096c0c2112e6e8de8e41c0f0df9dbf7146f4ba71 (released in v2). Mostly for cases where the last thing you do is not .send, but still. There's also this issue (again, not send).

So while I don't think this is necessary for fastify@3 it certainly doesn't hurt, and is a well-supported pattern. And seeing as it does make it work with fastify@4, I don't think there's any reason to land it? Even if the code is moved out of this repo later

[ro-savage]

@glasser - We'd love to see this merged. A small change from an awesome OSS contributor like @SimenB that allow Fastify v4 support would be extremely helpful to a lot of users of Apollo + Fastify.

Is there anything we can do to help this get merged?

[glasser]

Reviewing this is on my todo list, but every time I review a Fastify PR I basically have to re-learn how Fastify works from scratch (which is why we're passing that responsibility over to community members who actually use Fastify as part of AS4). But yes, high on my todo list, and certainly the contributor's reputation makes me suspect it will be fine :)

[glasser]

Published in v3.10.2.

[SimenB]

Thanks! ❤️

[natenrb9]

Should this theoretically enable support for Fastify v4?

I am getting this error with the newest version:
FastifyError: fastify-plugin: fastify-accepts - expected '3.x' fastify version, '4.4.0' is installed

Does something else need to be upgraded in the apollo-server dependencies to support Fastify v4?

[SimenB]

This is the only code change needed, but you also need to update the deps (which would be a breaking change, so won't happen in this repo). With yarn 2 or 3 like

{
  "resolutions": {
    "apollo-server-fastify/@fastify/accepts": "^4.0.0",
    "apollo-server-fastify/@fastify/cors": "^8.0.0"
  }
}

I have no idea how to do it for npm or pnpm, but there are probably ways (npm use overrides).

[glasser]

yes, as mentioned above (#6576 (comment)), we can't support multiple incompatible versions of Fastify from the same major version of the same apollo-server-fastify package. We are fixing this by decoupling "apollo server package version" from "apollo server web package X integration package version" in AS4 but will not be fixing it for AS3.

[natenrb9]

This is the only code change needed, but you also need to update the deps (which would be a breaking change, so won't happen in this repo). With yarn 2 or 3 like

{
  "resolutions": {
    "apollo-server-fastify/@fastify/accepts": "^4.0.0",
    "apollo-server-fastify/@fastify/cors": "^8.0.0"
  }
}

I have no idea how to do it for npm or pnpm, but there are probably ways (npm use overrides).

For anyone interested that works with npm to add Fastify v4 support.

I had to add this to my package.json, and then I ran npm update --save. I also had to update my npm version to support overrides.

"overrides": {
    "apollo-server-fastify": {
      "@fastify/accepts": "^4.0.0",
      "@fastify/cors": "^8.0.0"
    }
  },