Skip to content

Commit 7b1cfa8

Browse files
authored
[Node] Gracefully handle connection errors in the outbound network proxy (#2370)
## Motivation for the change, related issues As discovered in Automattic/studio#1491, adding a timeout before closing the network connection in the outbound network proxy prevents freezing the PHP runtime when the target cannot be reached. This PR adds such a timeout. The exact reason behind this behavior isn't well understood yet. The freeze is not deterministic or easy to reproduce, which is why this PR does not ship any tests. We should keep an eye on this and, if it ever comes up again, invest some time in a proper deep dive to understand all the factors at play. Related to Automattic/studio#1491 cc @gavande1 @mho22
1 parent dea8e0b commit 7b1cfa8

File tree

1 file changed

+27
-5
lines changed

1 file changed

+27
-5
lines changed

packages/php-wasm/node/src/lib/networking/outbound-ws-to-tcp-proxy.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
'use strict';
99

1010
import * as dns from 'dns';
11-
import * as util from 'node:util';
12-
import * as net from 'net';
1311
import * as http from 'http';
12+
import * as net from 'net';
13+
import * as util from 'node:util';
1414
import { WebSocketServer } from 'ws';
1515
import { debugLog } from './utils';
1616

@@ -146,6 +146,15 @@ async function onWsConnect(client: any, request: http.IncomingMessage) {
146146
return;
147147
}
148148

149+
// Validate port range
150+
if (reqTargetPort < 0 || reqTargetPort > 65535) {
151+
clientLog('Invalid port number: ' + reqTargetPort);
152+
// Send empty binary data to notify requester that connection failed
153+
client.send([]);
154+
client.close(3000);
155+
return;
156+
}
157+
149158
// eslint-disable-next-line prefer-const
150159
let target: any;
151160
const recvQueue: Buffer[] = [];
@@ -207,7 +216,11 @@ async function onWsConnect(client: any, request: http.IncomingMessage) {
207216
// Send empty binary data to notify requester that connection was
208217
// initiated
209218
client.send([]);
210-
client.close(3000);
219+
// Without this random timeout, PHP sometimes doesn't notice the socket
220+
// disconnected. TODO: figure out why.
221+
setTimeout(() => {
222+
client.close(3000);
223+
});
211224
return;
212225
}
213226
} else {
@@ -238,7 +251,16 @@ async function onWsConnect(client: any, request: http.IncomingMessage) {
238251
});
239252
target.on('error', function (e: any) {
240253
clientLog('target connection error', e);
241-
target.end();
242-
client.close(3000);
254+
client.send([]);
255+
// Without this random timeout, PHP sometimes doesn't notice the socket
256+
// disconnected. TODO: figure out why.
257+
setTimeout(() => {
258+
client.close(3000);
259+
try {
260+
target.end();
261+
} catch {
262+
// Ignore
263+
}
264+
});
243265
});
244266
}

0 commit comments

Comments
 (0)