vercel / next.js

The React Framework
https://nextjs.org
MIT License
127.55k stars 27.05k forks source link

Next 13 /_next/webpack-hmr pending indefinitely with custom server #50461

Open luca-vogels opened 1 year ago

luca-vogels commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: win32
      Arch: x64
      Version: Windows 10 Pro
    Binaries:
      Node: 18.15.0
      npm: N/A
      Yarn: N/A
      pnpm: N/A
    Relevant packages:
      next: 13.4.5-canary.0
      eslint-config-next: 13.4.4
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4

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

App directory (appDir: true)

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

https://github.com/luca-vogels/luca-vogels

To Reproduce

import http from "http";
import express, { NextFunction, Request, Response } from "express";
import next from "next";

const dev = true;
const HTTP_BIND = "0.0.0.0";
const HTTP_PORT = 80;

const nextApp = next({ dev });
const nextHandler = nextApp.getRequestHandler();
const nextUpgradeHandler = nextApp.getUpgradeHandler();

nextApp.prepare().then(async () => {

    const app = express();
    const server = http.createServer(app);

    // all frontend routes
    app.all('*', (req: Request | any, res: Response) => {    
        return nextHandler(req, res);
    });

    // all frontend routes (upgrade)
    server.on("upgrade", (req: Request, socket, head) => {
        nextUpgradeHandler(req, socket, head);
    });

    // start server
    server.listen(HTTP_PORT, HTTP_BIND, function(){
        console.log("Server running at "+HTTP_BIND+":"+HTTP_PORT+" in "+(dev ? "development" : "production")+" mode");
    });
});

Describe the Bug

Since Next 13, when using with a custom server (e.g. express), the route /_next/webpack-hmr gets no longer served (pending indefinitely) which breaks the hot-refresh functionality!

Next 12 worked flawlessly and the nextApp.getUpgradeHandler() wasn't even needed. Working example with Next 12:

import express, { NextFunction, Request, Response } from "express";
import next from "next";

const dev = true;
const HTTP_BIND = "0.0.0.0";
const HTTP_PORT = 80;

const nextApp = next({ dev });
const nextHandler = nextApp.getRequestHandler();

nextApp.prepare().then(async () => {
    const app = express();

    // all frontend routes
    app.all('*', (req: Request | any, res: Response) => {
        return nextHandler(req, res);
    });

    // start server
    app.listen(HTTP_PORT, HTTP_BIND, function(){
        console.log("Server running at "+HTTP_BIND+":"+HTTP_PORT+" in "+(dev ? "development" : "production")+" mode");
    });
});

Expected Behavior

Route /_next/webpack-hmr gets served again by the getRequestHandler() provided by next, so hot-refreshing works as expected. Worked in Next 12 as well.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

Node (custom server)

hesalx commented 1 year ago

The issue was introduced with https://github.com/vercel/next.js/pull/49805/files

It adds an extra proxy layer but does not proxy WebSockets. next-dev-server magically adds a WebSocket handler to the request's server such that if you use custom server (express or http.createServer) and pass req to the next.js handler (returned by .getRequestHandler()) you will have a WebSocket handler on your custom server.

However, since v13.4.3-canary.3 next.js forces creation of render-server instead, which is similar to a custom server in that it calls next({ ... }) and forwards requests to it, while the next({ ... }) you called simply proxies requests to render-server (render-server-standalone.ts). But render-server is it's own HttpServer. In this case next-dev-server adds WebSocket handler onto the HttpServer of render-server. And there is nothing that would forward WebSocket from your custom server to the render-server to even have a chance to be forwarded to next-dev-server.

render-server-standalone.ts should have had it's own version of what DevServer.setupWebSocketHandler does.

SuhyeongCho commented 1 year ago

I have same issue how can I fix it?

I also have custom server (server.js) with app rouing. (next@13.4.5)

thgh commented 1 year ago

Downgrade next to 13.3 works for me 🤷 (needed to add appDir: true in config)

w8ze-devel commented 1 year ago

Is there any better solution that downgrade ?

hesalx commented 1 year ago

If you find yourself in a situation where you can not downgrade (due to other fixes in 13.4.3+) and your local setup is barely usable (without HMR) then:

https://github.com/ds300/patch-package

Disclaimer: The maintenance of local patches is on you. If you can wait while on 13.4.2, better do so.

patches/next+13.4.6.patch (can be 13.4.3 to 13.4.6):

diff --git a/node_modules/next/dist/server/lib/render-server-standalone.js b/node_modules/next/dist/server/lib/render-server-standalone.js
index bc7de33..c960d94 100644
--- a/node_modules/next/dist/server/lib/render-server-standalone.js
+++ b/node_modules/next/dist/server/lib/render-server-standalone.js
@@ -11,6 +11,7 @@ Object.defineProperty(exports, "createServerHandler", {
 const _httpproxy = /*#__PURE__*/ _interop_require_default(require("next/dist/compiled/http-proxy"));
 const _jestworker = require("next/dist/compiled/jest-worker");
 const _utils = require("../../shared/lib/utils");
+const _log = require("../../build/output/log");
 function _interop_require_default(obj) {
     return obj && obj.__esModule ? obj : {
         default: obj
@@ -74,8 +75,28 @@ const createServerHandler = async ({ port , hostname , dir , dev =false , minima
         });
         return proxyServer;
     };
+
+    // Fix hot module replacement with a standalone server
+    let addedUpgradeListener = false
+    const setupWebSocketHandler = (req) => {
+        if (addedUpgradeListener) {
+            return;
+        }
+        let server = req?.socket?.server;
+        if (!server) {
+            _log.error(`Invalid IncomingMessage received, make sure http.createServer is being used to handle requests.`);
+        } else {
+            server.on("upgrade", async (req, socket, head) => {
+                const proxyServer = getProxyServer(req.url || '/')
+                proxyServer.ws(req, socket, head)
+            });
+            addedUpgradeListener = true;
+        }
+    }
+
     // proxy to router worker
     return async (req, res)=>{
+        setupWebSocketHandler(req);
         const urlParts = (req.url || "").split("?");
         const urlNoQuery = urlParts[0];
         // this normalizes repeated slashes in the path e.g. hello//world ->
chrisbenincasa commented 1 year ago

Can confirm the patch above worked for me (used pnpm patch instead of patch-package for the same effect)

guofei0723 commented 1 year ago

I have same issue. And it still exists after Next upgrade to 13.4.7

luca-vogels commented 1 year ago

Still not working with 13.4.7. I've even added the getUpgradeHandler() from Next but even with that the webpack-hmr doesn't get any response.

server.mts


const dev = Config.isDevMode();
const HTTP_BIND = process.env.HTTP_BIND || "0.0.0.0";
const HTTP_PORT = parseInt(process.env.HTTP_PORT || "80") || 80;

const app = express();
const server = http.createServer(app);

// ... custom routes

const nextApp = next({ dev, port: HTTP_PORT, httpServer: server });
const nextHandler = nextApp.getRequestHandler();
const nextUpgradeHandler = nextApp.getUpgradeHandler();

nextApp.prepare().then(async () => {

    // all frontend routes
    app.all('*', (req: Request | any, res: Response) => {
        return nextHandler(req, res);
    });

    // all frontend routes (upgrade)
    server.on("upgrade", (req: Request, socket, head) => {
        nextUpgradeHandler(req, socket, head);
    });

    // start server
    server.listen(HTTP_PORT, HTTP_BIND, () => {
        console.log("Server running at "+HTTP_BIND+":"+HTTP_PORT+" in "+(dev ? "development" : "production")+" mode");
    });
});
hesalx commented 1 year ago

@luca-vogels getRequestHandler() and getUpgradeHandler() don't do what you think they do if you have appDir. They actually point to completely different servers, with the latter being a no-op server, so using getUpgradeHandler() is futile. The issue is much deeper.

Instead of

server.ts -> NextServer

is it

server.ts -> Proxy -[getRequestHandler]-> custom handler -> httpProxy -> router worker -> real NextServer
server.ts -> Proxy -[getUpgradeHandler]-> dummy NextServer

WebSocket was just completely overlooked in this extra proxy layer. Someone with a deeper understanding of how and why all of this was implemented this way should have a look and come up with a proper fix.

Here is roughly what is missing to support getUpgradeHandler():

--- a/node_modules/next/dist/server/lib/render-server-standalone.js
+++ b/node_modules/next/dist/server/lib/render-server-standalone.js
@@ -75,7 +75,7 @@ const createServerHandler = async ({ port , hostname , dir , dev =false , minima
         return proxyServer;
     };
     // proxy to router worker
-    return async (req, res)=>{
+    return async (req, res, socket, head)=>{
         const urlParts = (req.url || "").split("?");
         const urlNoQuery = urlParts[0];
         // this normalizes repeated slashes in the path e.g. hello//world ->
@@ -90,7 +90,11 @@ const createServerHandler = async ({ port , hostname , dir , dev =false , minima
             return;
         }
         const proxyServer = getProxyServer(req.url || "/");
-        proxyServer.web(req, res);
+        if (head) {
+            proxyServer.ws(req, socket, head)
+        } else {
+            proxyServer.web(req, res);
+        }
         proxyServer.on("error", (err)=>{
             res.statusCode = 500;
             res.end("Internal Server Error");
diff --git a/node_modules/next/dist/server/next.js b/node_modules/next/dist/server/next.js
index 2e3acc4..78fcff7 100644
--- a/node_modules/next/dist/server/next.js
+++ b/node_modules/next/dist/server/next.js
@@ -302,6 +302,20 @@ function createServer(options) {
                                 return server.render(req, res, pathname, query, parsedUrl);
                             };
                         }
+                    case "getUpgradeHandler":
+                        {
+                            return ()=> {
+                                let handler;
+                                return async (req, socket, head) => {
+                                    if (shouldUseStandaloneMode) {
+                                        const standaloneHandler = await handlerPromise;
+                                        return standaloneHandler(req, undefined, socket, head);
+                                    }
+                                    handler = handler || server.getUpgradeHandler();
+                                    return handler(req, socket, head);
+                                };
+                            }
+                        }
                     default:
                         {
                             const method = server[propKey];
jacobsfletch commented 1 year ago

I'm experiencing the same issue with the Payload Custom Server Example which uses the Next.js App Router for its demonstration. It worked flawlessly with the Pages Router, but since upgrading to App Router, HMR no longer works. The app compiles in real-time but the browser requires a refresh to see the changes.

SuhyeongCho commented 1 year ago

still exists next@13.4.10

NoUnique commented 1 year ago

The custom server code below works in v13.4.12

// server.js

const fs = require('fs')
const next = require('next')
const https = require('https')
const { resolve } = require('path')
const { PHASE_PRODUCTION_SERVER } = require('next/constants')
const { PHASE_DEVELOPMENT_SERVER } = require('next/constants')
const { default: loadConfig } = require('next/dist/server/config')
const { default: DevServer } = require('next/dist/server/dev/next-dev-server')

const dev = process.env.NODE_ENV !== 'production'
const HOST = process.env.HOSTNAME || 'localhost'
const PORT = process.env.PORT || 8443

const httpsOptions = {
  key: fs.readFileSync('localhost-key.pem'),
  cert: fs.readFileSync('localhost.pem')
}

// hack1: set 'experimental' instead of 'next' for serverAction
// https://github.com/vercel/next.js/issues/49169#issuecomment-1542220461
process.env.__NEXT_PRIVATE_PREBUNDLED_REACT = 'experimental'

// hack2: create server without listeners to set as conf.httpServer
const server = https.createServer(httpsOptions)

// hack3: use DevServer for Webpack HMR socket handling
const createServer = dev ? options => new DevServer(options) : next

loadConfig(
  dev ? PHASE_DEVELOPMENT_SERVER : PHASE_PRODUCTION_SERVER,
  resolve('.'),
  undefined,
  undefined,
  false
).then(conf => {
  const app = createServer({
    dev,
    hostname: HOST,
    port: PORT,
    httpServer: server,
    conf
  })

  const handle = app.getRequestHandler()

  app.prepare().then(() => {
    server.on('request', (req, res) => {
      handle(req, res)
    })

    server.listen(PORT, err => {
      if (err) throw err
      console.log(`> HTTPS: Ready on https://${HOST}:${PORT}`)
    })
  })
})
luca-vogels commented 1 year ago

Issue got fixed in latest canary build next@13.4.14-canary.0

CMCDragonkai commented 1 year ago

No hacks necessary now?

CMCDragonkai commented 1 year ago

If I'm running a websocket server at the root of the app, do I have specially handle the upgrade request? Or is there a specific route that needs the specific upgrade handling?

rileysparsons commented 1 year ago

Issue got fixed in latest canary build next@13.4.14-canary.0

I can confirm this.

luca-vogels commented 1 year ago

If I'm running a websocket server at the root of the app, do I have specially handle the upgrade request? Or is there a specific route that needs the specific upgrade handling?

Haven't tested that but if it doesn't work have a look at lup-express-ws.

It handles upgrades just for your specific routes and passes the remaining ones on e.g. to next

CMCDragonkai commented 1 year ago

I'm using fastify and the fastify ws plugin. It doesn't appear to work because the ws plugin seems to takeover all the upgrade routes.

asrul10 commented 1 year ago

Just upgrade to next@13.4.19 and it's work

my custom server:

import express from "express";
import next from "next";
import { createProxyMiddleware } from "http-proxy-middleware";

const apiHost = "http://localhost:3002";
const port = parseInt(process.env.PORT, 10) || 3000;
const app = next({ dev: true });
const handle = app.getRequestHandler();

app.prepare().then(() => {
  const server = express();

  server.use(
    "/_next/webpack-hmr",
    createProxyMiddleware(
      "/_next/webpack-hmr",
      {
        ws: true,
        target: `ws://localhost:${port}`,
      }
    ),
  );

  server.use(
    "/api",
    createProxyMiddleware({
      target: apiHost,
      changeOrigin: true,
    }),
  );

  server.all("*", (req, res) => handle(req, res));

  server.listen(port, (err) => {
    if (err) throw err;
    console.log(`> Ready on http://localhost:${port}`);
  });
});
timmywil commented 1 year ago

13.4.19 does indeed seem to be working, but I think the custom server example needs to be updated to include getUpgradeHandler...

// ...
const handle = app.getRequestHandler()
const upgradeHandler = app.getUpgradeHandler()

app.prepare().then(() => {
  const server = createServer((req, res) => {
    const parsedUrl = parse(req.url!, true)
    handle(req, res, parsedUrl)
  })

  server.on('upgrade', upgradeHandler)

  server.listen(port)

// ...
timmywil commented 1 year ago

I take that back. The upgrade handler gets added automatically on the first request as before. However, I am adding my own websocket to a custom server (needs to be on the same port). I only handle upgrade requests that match a certain path, much like the HMR upgrade handler that only handles requests for _next/webpack-hmr.

But, commit 1398de9977b89c9c4717d26e213b52bc63ccbe7e introduced in 13.4.13 made it so the socket gets closed when not handled by HMR (before, it worked fine to have the HMR upgrade handler added to the server alongside my own). This is bad because there is now currently no clean way I can see to add your own socket to the routing server. I think the plan is to support upgrade requests in app/pages routes, but like I said the socket is now being closed instead of being allowed to continue.

I'm working around it for now with the following...

// ...
const handle = app.getRequestHandler()
const upgradeHandler = app.getUpgradeHandler()

app.prepare().then(() => {
  const server = createServer((req, res) => {
    const parsedUrl = parse(req.url!, true)
    handle(req, res, parsedUrl)
  })

  server.on('upgrade', (req, socket, head) => {
    const parsedUrl = parse(req.url!, true)
    if (parsedUrl.pathname === '/our-custom-socket-path') {
      return handleUpgradeOurselves(req, socket, head)
    }
    // Let next.js handle the rest
    return upgradeHandler(req, socket, head)
  })

  // Keep the next.js upgrade handler
  // from being added to our custom server so
  // sockets stay open even when not HMR.
  const originalOn = server.on.bind(server)
  server.on = function (event, listener) {
    if (event !== 'upgrade') {
      return originalOn(event, listener)
    }
    return server
  }

  server.listen(port)
// ...
timmywil commented 1 year ago

Sorry, that won't work either. It fixed my own websocket, but HMR stopped working. I had assumed I could call the upgradeHandler in the 'upgrade' event listener, but that's apparently not equivalent. ~If I want both, I'll have to downgrade to 13.4.12.~ 13.4.19 seems to have a HMR fix for custom server + app router (which I think is OP's issue), but I'm kind of stuck.

Fixing the user websocket regression would be a big bonus for me.

@ijjk I think all that would be needed to fix any user websockets would be to remove this line.

Side note: I think these are all related to HMR with a custom server + app router: #51162, #51028, #50881, #50400

CMCDragonkai commented 1 year ago

In my case my custom websocket path is the root of the app. Any ideas how to deal with this?

timmywil commented 1 year ago

I doubt everyone can use my solution, but I've been hitting so many issues with using a custom server lately (not just with HMR) that I decided to drop it. Instead, I switched to a separate HTTP server for my websocket on another port and am limiting the socket to a certain path. Then, I added a path rule to my load balancer to route all requests from my socket path to the socket server port. This seems to work fine and I've got HMR back. It's definitely more complicated, but I just don't think custom servers get enough testing, at least not yet with all the major refactors that have been happening.

jacobsfletch commented 1 year ago

Bumping from 13.4.8 to 13.4.19 appears to have fixed this for me.

roopakv commented 1 year ago

I have a weird bug where it isn't pending anymore with 13.4.19, but the page doesn't update 😅

I see the socket message come through and webpack pulls the latest updates for the page, but they arent actually hot reloaded / swapped

roopakv commented 1 year ago

Turns out my bug was fixed with 13.4.19 and the issue was caused by the nextjs sentry plugin.

Ended up not using it in the next.config.js for dev for now.

everlasting-neverness commented 1 year ago

I still have an issue with my custom websocket, instantiated with express-js by server.on("upgrade", ...) and latest (13.5.4) next-js version. Webpack-hmr seems to close my custom websocket's connection.

dsantos747 commented 1 year ago

I would also like to add I'm encountering this issue. I'm running next.js 13.5.4, with a REST server hosted on Flask. The Webpack-hmr request remains infinitely pending in my browser tools. Note, this does not have any impact on my hot-reload.

nalnir commented 1 year ago

Facing same issue in next 14.0.2. Webpack-hmr just never finishes.

avarayr commented 11 months ago

https://github.com/vercel/next.js/pull/58704 This PR should fix it, but Vercel team is not prioritizing it atm.