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

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

Open
1 task done
luca-vogels opened this issue May 28, 2023 · 31 comments
Open
1 task done

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

luca-vogels opened this issue May 28, 2023 · 31 comments
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template.

Comments

@luca-vogels
Copy link

Verify canary release

  • I verified that the issue exists in the latest Next.js 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)

@luca-vogels luca-vogels added the bug Issue was opened via the bug report template. label May 28, 2023
@github-actions github-actions bot added the area: app App directory (appDir: true) label May 28, 2023
@hesalx
Copy link

hesalx commented May 31, 2023

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
Copy link

I have same issue
how can I fix it?

I also have custom server (server.js) with app rouing. ([email protected])

@thgh
Copy link

thgh commented Jun 13, 2023

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

@w8ze-devel
Copy link

Is there any better solution that downgrade ?

@hesalx
Copy link

hesalx commented Jun 19, 2023

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
Copy link

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

@guofei0723
Copy link

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

@luca-vogels
Copy link
Author

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
Copy link

hesalx commented Jun 26, 2023

@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
Copy link
Contributor

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
Copy link

still exists [email protected]

@NoUnique
Copy link

NoUnique commented Aug 8, 2023

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
Copy link
Author

Issue got fixed in latest canary build [email protected]

@CMCDragonkai
Copy link

No hacks necessary now?

@CMCDragonkai
Copy link

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
Copy link

Issue got fixed in latest canary build [email protected]

I can confirm this.

@luca-vogels
Copy link
Author

luca-vogels commented Aug 9, 2023

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
Copy link

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
Copy link

asrul10 commented Aug 21, 2023

Just upgrade to [email protected] 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
Copy link
Contributor

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
Copy link
Contributor

timmywil commented Aug 21, 2023

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 1398de9 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
Copy link
Contributor

timmywil commented Aug 21, 2023

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
Copy link

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

@timmywil
Copy link
Contributor

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.

dougkulak added a commit to dougkulak/examples-next-prisma-websockets-starter that referenced this issue Aug 30, 2023
It seems next v13.4.3 added a breaking change that causes sockets to be closed when not handled by HMR. As a result, websocket connections fail to upgrade. I've applied a fix from the following thread to address this issue for now:

vercel/next.js#50461 (comment)
@jacobsfletch
Copy link
Contributor

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

@roopakv
Copy link

roopakv commented Sep 6, 2023

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
Copy link

roopakv commented Sep 13, 2023

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
Copy link

everlasting-neverness commented Oct 11, 2023

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
Copy link

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
Copy link

nalnir commented Nov 23, 2023

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

@avarayr
Copy link

avarayr commented Dec 28, 2023

#58704
This PR should fix it, but Vercel team is not prioritizing it atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests