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.

Sign up for GitHub

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

Jump to bottom

Fix GraphQL proxy in dev environments without IPv6 #8233

Merged
merged 3 commits into from
May 11, 2023
Merged

Fix GraphQL proxy in dev environments without IPv6 #8233

merged 3 commits into from
May 11, 2023

Conversation

chousheng
Copy link
Contributor

@chousheng chousheng commented May 5, 2023 •
edited
Loading

This PR fixes the problem that the GraphQL dev proxy fails (EADDRNOTAVAIL) when the IPv6 loopback interface is unavailable.

Why

I was following the tutorial (in Chapter 2 - Getting Dynamic) but got the proxy error below when accessing the scaffolded /posts route:

web | <e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:8910/graphql to http://[::1]:8911/ [EADDRNOTAVAIL] (https://nodejs.org/api/errors.html#errors_common_system_errors)

So I dived into it and found the problem was that the dev proxy tried to connect to the GraphQL server via IPv6 ::1 (see code here) and my dev environment did not have the IPv6 loopback interface enabled:

$ ip -6 a
$ # shows nothing

However, I could not enable IPv6 because I was using a VS Code Dev Container with Docker Desktop on macOS, where IPv6 had not been supported yet (see docker/for-mac#1432).

Here are some related problem reports:

Changes

Looking at the history, I found that PR #940 fixed an intermittent connection failure problem (ECONNRESET) but introduced this new problem (EADDRNOTAVAIL).

PR #940 had two changes:

  1. Add Connection: keep-live to the HTTP request header sending to the GraphQL server behind the proxy
  2. Change the target host from localhost (auto IPv4 or IPv6) to ::1 (IPv6 only).

Searching around, I believe the first change alone fixed the ECONNRESET problem, as reported here and here. That is, to avoid the new EADDRNOTAVAIL problem, we can safely revert the second change to use localhost (which can automatically resolve to either IPv4 or IPv6 loopback address depending on the environment). So, I revert the second change in this PR.

Besides, as a side effect, we no longer need to set RWJS_DEV_API_URL env var in Gitpod anymore. So, I remove the RWJS_DEV_API_URL settings in the GitPod config in this PR.

Testing

I have manually tested this PR using the rw-test-app (created by running yarn run build:test-project) in the environments below:

  • Gitpod
  • Dev Container with VS Code and Docker Desktop on macOS
  • Native macOS without containers

I would like to let CI test the rest.

Thanks the core team for making a welcoming community. This is my first PR to Redwood (also the first PR to the open-source world). I would appreciate any feedback!

@chousheng chousheng marked this pull request as draft May 5, 2023 19:33
@chousheng chousheng marked this pull request as ready for review May 5, 2023 19:39
@replay-io
Copy link

replay-io bot commented May 5, 2023 •
edited
Loading

18 replays were recorded for 7390490.

image 0 Failed
image 18 Passed

View test run on Replay ↗︎

@thedavidprice thedavidprice requested review from dthyresson and removed request for dthyresson May 6, 2023 00:29
@thedavidprice
Copy link
Contributor

Hi @chousheng Welcome to Redwood! For a first PR, you definitely took on a challenge. Well done. And huge thank you for the amount of QA testing — very helpful.

This PR only includes code changes for Webpack and GitPod. It's likely you didn't add additional files/commits or didn't push them to GitHub. Could you please double-check?

@jtoar we'll need to sync up and spend some time on this one.

@chousheng
Copy link
Contributor Author

Thank you @thedavidprice for the reminder! I just double-checked the files. That's all I changed. Actually, the major code change was a one-liner in the webpack config packages/core/config/webpack.development.js.

@dthyresson
Copy link
Contributor

@chousheng and @thedavidprice I recall this setting be changed once or twice before from localhost to :: etc. see #3434

I’m not sure my at this change without maybe some logic or detection is sufficient.

@chousheng
Copy link
Contributor Author

Hi @dthyresson, thanks for the comment.

I looked at PR #3434 as well. PR #3434, like this PR, ended up changing the default setting in Gitpod from ::1 to localhost (by adding a new env var RWJS_DEV_API_URL and setting it to http://localhost in .gitpod.yml). So I believe using localhost should be sound.

Besides, we are currently using :: as the listening address in the fastify GraphQL server (code here). Using :: means listening on all IPv6 and all IPv4 addresses on all major OSes (see here and here). So if we use localhost, we do not have to worry about localhost resolving to IPv4 127.0.0.1 or IPv6 ::1. Both cases work. But if we use ::1, we cannot support environments lacking IPv6.

@Tobbe
Copy link
Member

Tobbe commented May 6, 2023 •
edited
Loading

Related: #8019
(I remove the host config to use the default, which is localhost)

@jtoar
Copy link
Contributor

jtoar commented May 10, 2023

@Tobbe talking with @thedavidprice, if you feel like you understand the setup enough to make decisions could you take lead on this and also get in your Fastify PR? If not I'm happy to pair

@Tobbe Tobbe added the release:fix This PR is a fix label May 11, 2023
@Tobbe Tobbe merged commit 071f9c2 into redwoodjs:main May 11, 2023
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone May 11, 2023
@Tobbe
Copy link
Member

Tobbe commented May 11, 2023 •
edited
Loading

Thanks for your PR @chousheng! Great work 🙂

@chousheng
Copy link
Contributor Author

@Tobbe, thank you!

@Tobbe Tobbe mentioned this pull request May 11, 2023
Merged
jtoar pushed a commit that referenced this pull request May 11, 2023
@chousheng @Tobbe @jtoar
Fix GraphQL proxy in dev environments without IPv6 (#8233)
ee384c4 
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
dac09 added a commit to dac09/redwood that referenced this pull request May 12, 2023
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into feat/make-vi…
1dcb77a 
…te-default

* 'main' of github.com:redwoodjs/redwood: (23 commits)
  fix: remove react 17/18 warning (redwoodjs#8300)
  chore(release): tolerate lerna publish faliure
  Recover lost connection (redwoodjs#8284)
  chore(deps): update dependency @faker-js/faker to v8 (redwoodjs#8296)
  chore(release): better git commits during release
  feat: experimental - Studio Overview and Performance Widgets (redwoodjs#8292)
  fix(forms): disable webpack-dev-server overlay (redwoodjs#8298)
  Fix studio lint warning (redwoodjs#8297)
  Fastify server: Default to localhost (redwoodjs#8019)
  Fix GraphQL proxy in dev environments without IPv6 (redwoodjs#8233)
  fix(deps): update dependency @graphiql/plugin-explorer to v0.1.18 (redwoodjs#8290)
  chore(deps): update dependency supertokens-auth-react to v0.32.3 (redwoodjs#8289)
  Add `setup sentry` command (redwoodjs#7790)
  chore: readme update core team and all contributors (redwoodjs#8288)
  fix(deps): update nivo monorepo to ^0.83.0 (redwoodjs#8286)
  fix(deps): update dependency babel-plugin-polyfill-corejs3 to v0.8.1 (redwoodjs#8281)
  chore(deps): update dependency @replayio/playwright to v0.3.30 (redwoodjs#8282)
  fix(deps): update dependency webpack to v5.82.1 (redwoodjs#8283)
  Add epilogue to builders (redwoodjs#8285)
  feat(studio): v2 studio (redwoodjs#8173)
  ...
@jtoar jtoar modified the milestones: next-release, v5.2.0 May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
release:fix This PR is a fix
Projects
None yet
Milestone
v5.2.0
Development

Successfully merging this pull request may close these issues.
5 participants
@chousheng @thedavidprice @dthyresson @Tobbe @jtoar