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 server: Default to localhost #8019

Merged
merged 4 commits into from
May 11, 2023

Conversation

Tobbe
Copy link
Member

@Tobbe Tobbe commented Apr 5, 2023

The table here is good for understanding the host options https://www.fastify.io/docs/latest/Reference/Server/#listen

I chose to just remove the option, which will use the default, which is localhost. If someone prefers to be more explicit about it, let me know and I'll add it in. (See EDIT below)

I needed this change for #8002
It will also work towards making it possible to use RW in ipv6-only environments (at least it should help, more changes might be needed in other places)

EDIT:

I decided to keep the host config, but changed it to localhost, which is the default. Right now this is a bit redundant, but at some point we'll want to make this configurable (see #6362) and then we'll need it.

@jtoar
Copy link
Contributor

jtoar commented Apr 20, 2023

Maybe related: #6362

@Tobbe Tobbe added the release:chore This PR is a chore (means nothing for users) label May 11, 2023
@Tobbe Tobbe enabled auto-merge (squash) May 11, 2023 19:49
@Tobbe
Copy link
Member Author

Tobbe commented May 11, 2023

Merging this in alignment with discussions over at #8233 (comment)

@Tobbe Tobbe merged commit d609db6 into redwoodjs:main May 11, 2023
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone May 11, 2023
jtoar pushed a commit that referenced this pull request May 11, 2023
* Fastify server: Default to localhost

* explicitly set host to localhost

---------

Co-authored-by: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com>
dac09 added a commit to dac09/redwood that referenced this pull request May 12, 2023
…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)
  ...
@Tobbe Tobbe deleted the tobbe-fastify-server-host-localhost branch May 13, 2023 09:49
@jtoar jtoar modified the milestones: next-release, v5.2.0 May 17, 2023
@dennemark
Copy link
Contributor

dennemark commented May 22, 2023

The fastify link above states some exceptions, when Not to use localhost.
Unfortunately we are now running into issues with localhost being the default. We need to set 0.0.0.0
Any way to use the redwood.toml [api] host variable instead?

When deploying to a Docker, and potentially other, containers, it is advisable to listen on 0.0.0.0 because they do not default to exposing mapped ports to localhost:

fastify.listen({ port: 3000, host: '0.0.0.0' }, (err, address) => {
  if (err) {
    fastify.log.error(err)
    process.exit(1)
  }
})

@Josh-Walker-GM
Copy link
Collaborator

Josh-Walker-GM commented May 22, 2023

Also sorry @Tobbe we appear to have went backwards on this work when we added the fastify ejection and updated the CLI handlers to use the new fastify package:

host: process.env.NODE_ENV === 'production' ? '0.0.0.0' : '::',

This does make me wonder why then the change in this PR is causing issues? 🤔 At least for the current main branch CLI this is the case.

jtoar added a commit that referenced this pull request May 24, 2023
jtoar added a commit that referenced this pull request May 24, 2023
@steveoon
Copy link

After upgrade to 5.2.3, run yarn rw dev, and http://localhost:8911/graphql could not be accessed.

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

@Tobbe
Copy link
Member Author

Tobbe commented May 31, 2023

@steveoon Can you please post your full console output? And the output of yarn rw info.
Thank you 🙂

@jtoar
Copy link
Contributor

jtoar commented May 31, 2023

@steveoon we reverted this change in [v5.2.3])https://github.com/redwoodjs/redwood/releases/tag/v5.2.3). There was a span of time between v5.2.0 and v5.2.3 (a few days really) where this functionality was in a stable release—did you make a change to api or web hosts in that span in redwood.toml?

@jtoar jtoar modified the milestones: v5.2.0, v6.0.0 Jun 2, 2023
jtoar added a commit that referenced this pull request Jun 28, 2023
jtoar added a commit that referenced this pull request Jun 28, 2023
@jtoar jtoar modified the milestones: v6.0.0, v7.0.0 Jun 28, 2023
jtoar added a commit that referenced this pull request Jun 28, 2023
* Revert "Reserve h for help, and format docs (#8407)"

This reverts commit 70b7c4d.

* Revert "feat(cli): Enable `host` option for `yarn rw serve` (#8385)"

This reverts commit 26ad3e2.

* Revert "Fastify server: Default to localhost (#8019)"

This reverts commit d609db6.
@jtoar jtoar modified the milestones: SSR, v7.0.0 Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants