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

Custom server breaks with app directory in development #50400

Closed
1 task done
cjonesdoordash opened this issue May 26, 2023 · 11 comments
Closed
1 task done

Custom server breaks with app directory in development #50400

cjonesdoordash opened this issue May 26, 2023 · 11 comments
Labels
bug Issue was opened via the bug report template. locked

Comments

@cjonesdoordash
Copy link

cjonesdoordash commented May 26, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 22.4.0: Mon Mar  6 21:00:17 PST 2023; root:xnu-8796.101.5~3/RELEASE_X86_64
    Binaries:
      Node: 16.18.0
      npm: 8.19.2
      Yarn: 1.22.5
      pnpm: 6.23.6
    Relevant packages:
      next: 13.4.4
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 4.9.5

Which area(s) of Next.js are affected? (leave empty if unsure)

Custom server

Link to the code that reproduces this issue or a replay of the bug

https://github.com/cjonesdoordash/nextjs-custom-server-issue

To Reproduce

You can validate that the linked repo works correctly in both development and production mode in its current state.

To reproduce the issue just add an empty app folder at the root level then run yarn dev and the issue will show up. Then you can run yarn build && yarn start and validate that the issue doesn't reproduce.

Describe the Bug

This issue only reproduces when the following criteria are met

  • an app folder is added (can be empty)
  • next is ran in development mode

When the above criteria are met you lose functionality on the request object that previously existed such as adding elements to the request or using express specific methods. Examples for both of these issues are provided in the linked repo. (note the app folder isn't added so you can validate that the issue doesn't repro without it)

Expected Behavior

You can still access express based elements on the request regardless of app directory usage and if the app is being ran in development or production mode.

Which browser are you using? (if relevant)

N/A

How are you deploying your application? (if relevant)

N/A

@cjonesdoordash cjonesdoordash added the bug Issue was opened via the bug report template. label May 26, 2023
@anulman
Copy link

anulman commented May 28, 2023

Wondering if this is related to why HMR does not work for custom servers with an app/ directory. The server seems to successfully upgrade the socket, but in my experience the client remains in a pending state and never gets the HTTP 101 that confirms the upgrade.

@paavole
Copy link

paavole commented May 29, 2023

I think I am facing same issue. We run NextJs with custom server and install bunch of ExpressJS middlewares which we access in NextJs pages via req: NextRequest. As per 13.4.3 they are now all gone. And so is the req.app.

@cjonesdoordash
Copy link
Author

Still seeing this in the latest canary 13.4.6-canary.2. Any chance there's been any movement on any of these seemingly related custom server issues?

@cjonesdoordash
Copy link
Author

Looks like this is caused directly because of the proxy-server introduced in #49805. It seems like the introduction of this proxy server has caused a number of issues (Sentry / HMR / custom request functionality) is there perhaps a different way the original issue can be sorted without using a proxy server? @shuding as the original implementer.

@hazzzi
Copy link

hazzzi commented Jul 3, 2023

It seems like this issue also exists in version 13.4.8. I had no choice but to go back from the app router to the pages router.

@Multiply
Copy link

Multiply commented Jul 7, 2023

This will also block our gradual adoption of the app directory, as we rely heavily on "global" data via the ctx.req.

We want to move away from this pattern by using the app directory, but if the old behaviour is not possible while moving over, it's going to be very difficult to upgrade.

@gidsola
Copy link

gidsola commented Jul 28, 2023

13.4.12 this issue is still relevant.
Edit: I should have added that this exists on node:http/s implementations also.

    const
        fs = require('node:fs'),
        http = require('node:http'),
        https = require('node:https'),
        next = require('next'),
        options = {
          ciphers: process.env.ciphers,
          maxVersion: process.env.maxVersion,
          minVersion: process.env.minVersion,
          key: fs.readFileSync(process.cwd() + `/src/ssl/private.key`),
          cert: fs.readFileSync(process.cwd() + `/src/ssl/certificate.crt`),
          ca: [fs.readFileSync(process.cwd() + `/src/ssl/ca_bundle.crt`)],
          maxHeaderSize: parseInt(process.env.maxHeaderSize),
          requestTimeout: parseInt(process.env.requestTimeout),
          headersTimeout: parseInt(process.env.headersTimeout),
          sessionTimeout: parseInt(process.env.sessionTimeout),
          requestCert: (process.env.requestCert == "true") ? true : false,
          enableTrace: (process.env.insecureHTTPParser == "true") ? true : false,
          rejectUnauthorized: ((process.env.rejectUnauthorized == "true") ? true : false),
          insecureHTTPParser: ((process.env.insecureHTTPParser == "true") ? true : false)
        },
        hostname = 'example.com',
        port = 443,
        dev = process.env.NODE_ENV !== 'production',
        app= next({ dev, hostname, port }),
        handle = app.getRequestHandler();
      app
        .prepare()
        .then(() => {
          https.createServer(options)
            .listen(port, function isListening() { console.log('.. listening ..') })
          

@cjonesdoordash
Copy link
Author

Closed as this issue has been resolved

@Multiply
Copy link

Multiply commented Oct 6, 2023

@cjonesdoordash how was this solved?

@cjonesdoordash
Copy link
Author

@Multiply Upstream Next.js change that made the context.req object actually reflect the correct express request. We're currently using 13.5.1 and everything works as expected there within the pages directory.

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. locked
Projects
None yet
Development

No branches or pull requests

6 participants