Some errors are not handled correctly for proxied requests

[OliverJAsh]

• Operating System: macOS 10.15.2
• Node Version: 12.8.1
• NPM Version: 6.10.2
• webpack Version: see package.json below
• webpack-dev-server Version: see package.json below
• Browser: N/A

• This is a bug
• This is a modification request

Code

// webpack.config.js
module.exports = {
    devServer: {
        port: 4000,
        proxy: {
            '/api': 'http://localhost:3000'
        }
    }
}

// index.js
const express = require("express");

const app = express();

app.get("*", (req, res) => {
  res.send("foo");
});

// error handling middleware
app.use((error, req, res, next) => {
  res.send("my custom error page");
});

app.listen(3000);

// package.json
{
  "dependencies": {
    "express": "^4.17.1",
    "webpack": "^4.41.5",
    "webpack-cli": "^3.3.10",
    "webpack-dev-server": "^3.10.1"
  }
}

Expected Behavior

1. Run node index.js
2. Run webpack-dev-server
3. curl "localhost:4000/api/%"

The response should be "my custom error page", indicating that the Express error handling middleware for the API server was used.

Actual Behavior

The response is URIError: Failed to decode param …, indicating that the Express error handling middleware for the API server was not used.

I would not expect webpack-dev-server to change how errors are handled for proxied requests.

Technical notes

I played around with using http-proxy-middleware on its own to see if the root issue was inside there, but using the following code, I could not reproduce this issue:

const httpProxyMiddleware = require("http-proxy-middleware");
const express = require("express");

{
  const app = express();

  app.get("*", (req, res) => {
    res.send("foo");
  });

  app.use((error, req, res, next) => {
    res.send("my custom error page");
  });

  app.listen(3000);
}

{
  const app = express();

  app.use(httpProxyMiddleware({ target: "http://localhost:3000" }));

  app.listen(4000);
}

For Bugs; How can we reproduce the behavior?

See above

For Features; What is the motivation and/or use-case for the feature?

N/A

[alexander-akait]

I would not expect webpack-dev-server to change how errors are handled for proxied requests.

We don't do it, can you create minimum reproducible test repo?

[OliverJAsh]

Absolutely: https://github.com/OliverJAsh/webpack-dev-server-uri-error-test

[alexander-akait]

Thanks, i will look at this in near future

[alexander-akait]

WIP

[alexander-akait]

It is problem from express, we cannot do anything here, for example write curl "localhost:3000/" and curl "localhost:3000/%", in theory you can rewrite own code like:

app.use((error, req, res, next) => {
  var err = null;
  try {
    decodeURIComponent(req.path)
  }
  catch(e) {
    err = e;
  }
  if (err){
    res.send("my custom error page");
  }
  next();
  
 
});

But we can't do it because we use a lot of middlewares which doesn't handle that error, sorry

Feel free to feedback, anyway what is use case, it is broken URL

[OliverJAsh]

what is use case, it is broken URL

I want my development environment to behave the same as production. I'm working on the error page for invalid URLs but because of this bug I cannot see that error page in development (i.e. when using this module).

[alexander-akait]

You can use the before option and setup own implemented error middleware, it is edge case and that’s how express works out of the box, if you need change something you can setup own middleware

[OliverJAsh]

setup own implemented error middleware

I have already done this on my own server. The issue is that the proxy used by webpack-dev-server does not correctly pass the request through to my server.

[alexander-akait]

Express correctly parse your request, you have invalid URL and it is expected behavior, if you need change that behavior please use the before option with custom middleware

[OliverJAsh]

If you make a request to the server (skipping the proxy), you will see the error page. ✅

$ curl "localhost:3000/%"
my custom error page

If you make a request to the proxy, you will not see the error page. ❌

$ curl "localhost:4000/api/%"
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>URIError: Failed to decode param &#39;/%&#39;<br> &nbsp; &nbsp;at decodeURIComponent (&lt;anonymous&gt;)<br> &nbsp; &nbsp;at decode_param (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/layer.js:172:12)<br> &nbsp; &nbsp;at Layer.match (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/layer.js:123:27)<br> &nbsp; &nbsp;at matchLayer (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:574:18)<br> &nbsp; &nbsp;at next (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:220:15)<br> &nbsp; &nbsp;at expressInit (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/middleware/init.js:40:5)<br> &nbsp; &nbsp;at Layer.handle [as handle_request] (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/layer.js:95:5)<br> &nbsp; &nbsp;at trim_prefix (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:317:13)<br> &nbsp; &nbsp;at /Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:284:7<br> &nbsp; &nbsp;at Function.process_params (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:335:12)</pre>
</body>
</html>

Do you see what's wrong? The proxy does not preserve behaviour of the server.

[alexander-akait]

Do you see error stack? It is from express because it is default behavior

[OliverJAsh]

I tried to workaround this bug by using the before option, as you suggested @evilebottnawi:

module.exports = {
  devServer: {
    before: (app, server, compiler, proxy) => {
      // Forward error requests to the proxy so it can handle them.
      app.use((error, req, res, next) => proxy(req, res, next));
    },
  }
};

… but unfortunately this is not possible—the before function does not receive the proxy middleware.

In any case, I still believe this is something that should be handled by webpack-dev-server, so users don't have to think about it. I did some digging and I have a suggestion for how to do this.

Consider a simple server and proxy:

const httpProxyMiddleware = require("http-proxy-middleware");
const express = require("express");

{
  const app = express();

  app.get("*", (req, res) => {
    res.send("my home page");
  });

  app.use((error, req, res, next) => {
    res.send("my custom error page");
  });

  app.listen(3000);
}

{
  const app = express();

  app.all("*", (req, res, next) => {
    next();
  });

  const proxy = httpProxyMiddleware({ target: "http://localhost:3000" });
  app.use(proxy);

  app.listen(4000);
}

If you make a request to the server (skipping the proxy), you will see the error page. ✅

$ curl "localhost:3000/%"
my custom error page

If you make a request to the proxy, you will not see the error page. ❌

$ curl "localhost:4000/%"
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>URIError: Failed to decode param &#39;/%&#39;<br> &nbsp; &nbsp;at decodeURIComponent (&lt;anonymous&gt;)<br> &nbsp; &nbsp;at decode_param (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/layer.js:172:12)<br> &nbsp; &nbsp;at Layer.match (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/layer.js:123:27)<br> &nbsp; &nbsp;at matchLayer (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:574:18)<br> &nbsp; &nbsp;at next (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:220:15)<br> &nbsp; &nbsp;at expressInit (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/middleware/init.js:40:5)<br> &nbsp; &nbsp;at Layer.handle [as handle_request] (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/layer.js:95:5)<br> &nbsp; &nbsp;at trim_prefix (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:317:13)<br> &nbsp; &nbsp;at /Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:284:7<br> &nbsp; &nbsp;at Function.process_params (/Users/oliverash/Development/webpack-dev-server-uri-error-test/node_modules/express/lib/router/index.js:335:12)</pre>
</body>
</html>

As you can see, errors are being handled by the proxy server rather than the server itself. We would rather just forward these requests so they can be handled by the original server.

All we have to do is this:

   const proxy = httpProxyMiddleware({ target: "http://localhost:3000" });
+  // Forward error requests to the proxy so it can handle them.
+  app.use((error, req, res, next) => proxy(req, res, next));
   app.use(proxy);

Now, if you make a request to the proxy, you will see the error page. ✅

$ curl "localhost:4000/%"
my custom error page

This change would be very easy to incorporate into webpack-dev-server. If I created a PR for this, would you be happy to merge so we can fix this issue?

Potentially related: chimurai/http-proxy-middleware#311

[alexander-akait]

Okay, send a PR, don't forget add tests