Proxy Failure on WS Proxy Server Connection Close

[mjrussell]

HTPM just got merged into webpack(webpack/webpack-dev-server#359) which is awesome because it enables things like native websockets which didn't work previously.

I was just taking it for a spin and everything was working great until I took down the backend server before shutting down the dev-server. The logic in my front-end is such that after the socket dies it will recheck if it is logged in (hence the /api/users/me check). I can work on an example but it may take me a little while, wondering if it might be obvious from these logs + stack trace:

[HPM] Upgrading to WebSocket
[HPM] Client disconnected
[HPM] Proxy error: ECONNREFUSED. 127.0.0.1 -> "127.0.0.1:9020/api/users/me"
[HPM] Upgrading to WebSocket
./node_modules/webpack-dev-server/node_modules/http-proxy-middleware/lib/handlers.js:8
        res.writeHead(500);
            ^

TypeError: res.writeHead is not a function
    at ProxyServer.proxyError (./node_modules/webpack-dev-server/node_modules/http-proxy-middleware/lib/handlers.js:8:13)
    at ProxyServer.emit (./node_modules/webpack-dev-server/node_modules/http-proxy-middleware/node_modules/http-proxy/node_modules/eventemitter3/index.js:117:27)
    at ClientRequest.onOutgoingError (./node_modules/webpack-dev-server/node_modules/http-proxy-middleware/node_modules/http-proxy/lib/http-proxy/passes/ws-incoming.js:153:16)
    at emitOne (events.js:77:13)
    at ClientRequest.emit (events.js:169:7)
    at Socket.socketErrorListener (_http_client.js:259:9)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at emitErrorNT (net.js:1257:8)
    at doNTCallback2 (node.js:441:9)
    at process._tickCallback (node.js:355:17)

Im totally expecting the proxy to fail and even loudly complain, but I wouldn't expect it to crash the dev-server. If there's some hardening that must be done on the webpack-dev-server itself instead we can get a PR there.

[chimurai]

Thanks for reporting the issue.

Looks like the default error handler doesn't take care off WebSocket errors... (only http)
I'll have to take a look how to catch those properly.

[chimurai]

The main issue is when a WebSocket is established and an error occurs in the scenerio you described; the error-handler currently responds by returning a http 500 response. Which is fine in case of a http(s) error, but it will not work because the writeHead method doesn't exist for WebSockets.

I'm hoping @mjrussell might be more knowledgeable on how to handle WebSocket errors properly, since I'm not really sure.

Summing up the information I've encountered:

karma
https://github.com/karma-runner/karma/blob/ae05ea496b8fff1a316387f0b5919de673c5e274/lib/middleware/proxy.js#L60
They just call res.destroy() and log different messages for http(s)- and websocket errors

Destroy any sockets that are currently in use by the agent.
https://nodejs.org/dist/latest-v4.x/docs/api/http.html#http_agent_destroy

w3 / mdn
http://tools.ietf.org/html/rfc6455#page-41
https://www.w3.org/TR/websockets/#closeWebSocket
https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent#Status_codes
close(1006) WebSocket with code 1006.

mdn

A WebSocket server that abruptly closed the connection after successfully completing the opening handshake.

rfc6455

 1006 is a reserved value and MUST NOT be set as a status code in a
 Close control frame by an endpoint.  It is designated for use in
 applications expecting a status code to indicate that the
 connection was closed abnormally, e.g., without sending or
 receiving a Close control frame.

[mjrussell]

@chimurai nice summary! I would think that the proxy should just let the connection terminate, which I guess would be res.destroy().

I wouldn't expect the middleware to send an extra code or anything in the socket that the server itself didn't send, especially because sometimes people may run their own protocol/codes inside the socket.

The client side should be able to handle this as an unexpected error in the onError callback.

I do also like the extra logging done in the karma proxy log.debug('failed to proxy %s (browser hung up the socket)', req.url)

[chimurai]

@mjrussell can you verify if this patch fixes the issue?

[mjrussell]

@chimurai will do tomorrow, thanks for working this!

[chimurai]

That would be great!

I kept the changes tiny, so I won't disturb the current functionality too much.
Tried destroying the response onError, but some tests were failing. Need to investigate why it is happening.

Maybe I'll refactor error handling for version 1.0, so it will have proper error handling and logging.

[chimurai]

published fix in v0.9.1
https://github.com/chimurai/http-proxy-middleware/releases/tag/v0.9.1

[yoyo837]

"http-proxy-middleware": "0.19.1",

Same here.

After the server is shut down, the proxy will not crash immediately. Crash after trying to reconnect several times.