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

How to use HTTP/2 with React.JS / Next.JS!?! #4253

Closed
khteh opened this issue Sep 28, 2023 · 13 comments · Fixed by vercel/next.js#56768
Closed

How to use HTTP/2 with React.JS / Next.JS!?! #4253

khteh opened this issue Sep 28, 2023 · 13 comments · Fixed by vercel/next.js#56768

Comments

@khteh
Copy link

khteh commented Sep 28, 2023

Version

20.3.1

Platform

Linux khteh-p17-2i 6.2.0-33-generic nodejs/node#33-Ubuntu SMP PREEMPT_DYNAMIC Tue Sep 5 14:49:19 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

React.JS, Next.JS

What steps will reproduce the bug?

Reference: https://github.com/vercel/next.js/blob/canary/examples/with-http2/server.js

const app = next({ dev, hostname, port, dir: __dirname })
const server = http2.createSecureServer({
  key: fs.readFileSync('server.key'),
  cert: fs.readFileSync('server.crt')
})
app.prepare().then(() => {
})
    server.on('request', (req, res) => {
      console.log(`request ${req.url}`)
        app.render(req, res, req.url || '/', req.query)
      })    
    server.listen(port, (err) => {
      if (err) {
        console.error('Failed to start server', err)
        process.exit(1)
      } else {
        console.log(`HTTP/2 server listening on port: ${port}`)
      }
    })
}).catch(err => next(err))

error:

- error unhandledRejection: TypeError: Cannot read properties of undefined (reading 'end')

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

No response

What do you see instead?

- error unhandledRejection: TypeError: Cannot read properties of undefined (reading 'end')

Additional information

No response

@mscdex mscdex transferred this issue from nodejs/node Sep 28, 2023
@mcollina
Copy link
Member

Thanks for reporting, I would recommend you to open an issue on the Next.js tracker.

However, maybe @ovflowd could help too.

@ovflowd
Copy link
Member

ovflowd commented Sep 28, 2023

This seems to be a supported Next.js example, I am keen in thinking that this is a regression on Next.js

@khteh would you mind opening an issue on vercel/next.js and check if they rule this issue out on their side first?

@khteh
Copy link
Author

khteh commented Sep 29, 2023

Done!

@himself65
Copy link
Member

upstream vercel/next.js#50270

@himself65
Copy link
Member

I think we can close this issue since next.js doesn't care about custom server: vercel/next.js#8625 (comment)

@himself65
Copy link
Member

Hey, can you use the latest node 20 and this patch to see if the bug is fixed?

vercel/next.js#56768

@khteh
Copy link
Author

khteh commented Oct 13, 2023

 ⨯ TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at sendRenderResult (/usr/src/nextjsapp/node_modules/next/dist/server/send-payload/index.js:81:13)
 ✓ Compiled /_error in 1401ms (4915 modules)
TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at sendRenderResult (/usr/src/nextjsapp/node_modules/next/dist/server/send-payload/index.js:81:13)
TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at sendRenderResult (/usr/src/nextjsapp/node_modules/next/dist/server/send-payload/index.js:81:13)
 ⨯ unhandledRejection: TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at NextCustomServer.requestHandlerImpl [as requestHandler] (/usr/src/nextjsapp/node_modules/next/dist/server/lib/router-server.js:413:17)
 ⨯ unhandledRejection: TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at NextCustomServer.requestHandlerImpl [as requestHandler] (/usr/src/nextjsapp/node_modules/next/dist/server/lib/router-server.js:413:17)

@himself65
Copy link
Member

weird, I did see any usage of this function

@mcollina
Copy link
Member

I can 100% confirm this is a bug outside of Node.js.
More specifically, Next.js uses the compression module that monkeypatches internals and call private methods inside OutgoingMessage.
compression is part of express, which does not support HTTP/2.

My 2 cents: use libraries that do not monkey patch / depends on Node.js internals.

cc @styfle @Ethan-Arrowood to coordinate a fix on the Next.js side.

kodiakhq bot pushed a commit to vercel/next.js that referenced this issue Oct 13, 2023
### What?

If the custom server uses `app.render(xxx)` will render the normal js files as server components in dev mode and cause 404 error when loading every non-HTML file.

### How?

Fixes nodejs/help#4253, fixes #50270
@ovflowd
Copy link
Member

ovflowd commented Oct 13, 2023

Apparently they updated their http2 example with an approach that seems to be working.

But indeed, I wonder if they could fix this upstream and not monkeypatch said module.

@himself65
Copy link
Member

 ⨯ TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at sendRenderResult (/usr/src/nextjsapp/node_modules/next/dist/server/send-payload/index.js:81:13)
 ✓ Compiled /_error in 1401ms (4915 modules)
TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at sendRenderResult (/usr/src/nextjsapp/node_modules/next/dist/server/send-payload/index.js:81:13)
TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at sendRenderResult (/usr/src/nextjsapp/node_modules/next/dist/server/send-payload/index.js:81:13)
 ⨯ unhandledRejection: TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at NextCustomServer.requestHandlerImpl [as requestHandler] (/usr/src/nextjsapp/node_modules/next/dist/server/lib/router-server.js:413:17)
 ⨯ unhandledRejection: TypeError: this._implicitHeader is not a function
    at Http2ServerResponse.end (/usr/src/nextjsapp/node_modules/next/dist/compiled/compression/index.js:22:749)
    at NextCustomServer.requestHandlerImpl [as requestHandler] (/usr/src/nextjsapp/node_modules/next/dist/server/lib/router-server.js:413:17)

can you turn off the compression? I think next.js already know the bug on express

https://github.com/vercel/next.js/blob/fe0bfbf9118cef355fd6f2f6e667deb57ad66e69/examples/with-http2/next.config.js#L1-L7

@khteh
Copy link
Author

khteh commented Oct 14, 2023

can you turn off the compression? I think next.js already know the bug on express

It works. However,
(1) What are the implications of turning off compression?
(2) How to use logger with this library (morgan, winston). I can't use server.use :

 ⨯ unhandledRejection: TypeError: server.use is not a function

(3) How to listen on certain URLs and respond immediately? I could do the following with Express:

  expressApp.get('/health/ready', function (req, res, next) {
    res.send('OK')
  })
  expressApp.get('/health/live', function (req, res, next) {
    res.send('OK')
  })

(4) Does it work behind any public cloud load balancers? Does it trust the incoming X-FORWARDED-<foo> headers?
(5) Does it support stream?
These are the things which I have been using with Express and SPDY server:

  expressApp.use(express.json())
  expressApp.use(express.urlencoded({ extended: false }))
  expressApp.use(cookieParser())
  expressApp.use(express.static(path.join(__dirname, 'public')))
  //expressApp.use(helmet()) adding set of security middlewares. This breaks!
  expressApp.use(cors()) // enable all CORS request
  expressApp.set('port', port)
  expressApp.use(compression({ filter: shouldCompress }))
  expressApp.enable("trust proxy")
  expressApp.get('/index', (req, res) => {
    return app.render(req, res, '/', req.query)
  })
  /* k8s liveness check */
  expressApp.get('/health/ready', function (req, res, next) {
    res.send('OK')
  })
  expressApp.get('/health/live', function (req, res, next) {
    res.send('OK')
  })
    if (config.util.getEnv('NODE_ENV') === 'development')
      expressApp.use(
        morgan('combined', {
          skip: function (req, res) {
            return res.statusCode < 400
          },
        })
      )
    // log all requests to access.log
    expressApp.use(
      morgan(format, {
        stream: accessLogStream,
        skip: function (req, res) {
          return req?.url?.includes('health') || req?.url?.includes('robots.txt') || req?.url?.includes('/_next') || req?.url?.includes('/images') || req?.url?.includes('/icons')
        }
      }))

How does it look like if I switch over to your HTTP2 server? Are there any equivalents from your library? The reason why I am considering your HTTP2 library is to reduce / remove the layers of SPDY -> Express -> Next.JS.

@balazsorban44
Copy link

I think we can close this issue since next.js doesn't care about custom server: vercel/next.js#8625 (comment)

I would like to say that my wording was probably incorrect there. The custom server support has been basically feature complete and fairly stable for years.

The thing there is that since server.ts falls outside Next.js, it doesn't come with the same DX. The rest could be converted to TS easily.

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

Successfully merging a pull request may close this issue.

5 participants