Skip to content

Commit d489d25

Browse files
authored
fix: close proxy websocket upon TCP connection closing (#684)
This fixes a bug where clients using `neondatabase/serverless` would have their code hang for 60s upon calling `client.end()`. Essentially the client closes the (proxied) TCP socket, and we need to ensure that the websocket is closed as well. Otherwise the client keeps blocked waiting for the websocket to terminate. Added an integration test with such a case (times out without the patch). Unfortunately, the test requires bumping node, so I did that as well, will check to ensure it doesn't impact anything else.
1 parent 44fd128 commit d489d25

File tree

10 files changed

+378
-8
lines changed

10 files changed

+378
-8
lines changed

.tool-versions

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
rust 1.84.1
22
elixir 1.18.2-otp-27
3-
nodejs 18.13.0
4-
erlang 27.2
3+
nodejs 19.9.0
4+
erlang 27.2

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2.5.6
1+
2.5.7

lib/supavisor_web/ws_proxy.ex

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ defmodule SupavisorWeb.WsProxy do
4646
{[{:binary, bin}], state}
4747
end
4848

49+
def websocket_info({:tcp_closed, socket}, %{socket: socket}) do
50+
{:stop, :normal}
51+
end
52+
4953
def websocket_info(msg, state) do
5054
Logger.error("Undefined websocket_info msg: #{inspect(msg, pretty: true)}")
5155
{:ok, state}

test/integration/external_test.exs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@ defmodule Supavisor.Integration.ExternalTest do
2525
end
2626
end
2727

28+
describe "neondatabase/serverless.js" do
29+
@describetag library: "serverless.js", suite: "js"
30+
31+
@tag runtime: "node", mode: "session"
32+
test "Node session", ctx do
33+
assert_run(ctx, ~w[neon_serverless/index.js])
34+
end
35+
36+
@tag runtime: "node", mode: "transaction"
37+
test "Node transaction", ctx do
38+
assert_run(ctx, ~w[neon_serverless/index.js])
39+
end
40+
end
41+
2842
describe "Postgres.js" do
2943
@describetag library: "postgres.js", suite: "js"
3044

@@ -65,9 +79,10 @@ defmodule Supavisor.Integration.ExternalTest do
6579

6680
env =
6781
[
82+
# {"NODE_DEBUG", "*"},
6883
{"PGMODE", ctx.mode},
6984
{"PGDATABASE", ctx.db},
70-
{"PGHOST", "localhost"},
85+
{"PGHOST", "127.0.0.1"},
7186
{"PGPORT", to_string(port(ctx.mode))},
7287
{"PGUSER", ctx.user},
7388
{"PGPASS", "postgres"},
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { t, nt, ot } from '../shared/test.js' // eslint-disable-line
2+
import { Client, neonConfig } from "@neondatabase/serverless";
3+
import { WebSocket } from 'ws';
4+
globalThis.WebSocket = WebSocket;
5+
6+
const connectionString = `postgres://${encodeURIComponent(process.env.PGUSER)}:${encodeURIComponent(process.env.PGPASS)}@${process.env.PGHOST}:${process.env.PGPORT}/${process.env.PGDATABASE}${process.env.PGSSL ? '?ssl=true' : ''}`;
7+
8+
neonConfig.wsProxy = (_host, _port) => `127.0.0.1:4000/v2/`
9+
neonConfig.useSecureWebSocket = false;
10+
const client = new Client(connectionString);
11+
12+
t('Connect, run simple query, disconnect', async () => {
13+
await client.connect();
14+
const result = await client.query("SELECT 1 as one");
15+
await client.end();
16+
return [1, result.rows[0]["one"]];
17+
});

test/integration/js/package-lock.json

Lines changed: 251 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integration/js/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
"test:postgres": "node ./postgres/index.js"
99
},
1010
"dependencies": {
11-
"postgres": "^3.4.5"
11+
"@neondatabase/serverless": "^1.0.1",
12+
"postgres": "^3.4.5",
13+
"ws": "^8.18.3"
1214
}
1315
}

test/integration/js/postgres/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { t, nt, ot } from './test.js' // eslint-disable-line
1+
import { t, nt, ot } from '../shared/test.js' // eslint-disable-line
22
import net from 'node:net'
33
import fs from 'node:fs'
44
import crypto from 'node:crypto'

test/integration/js/postgres/test.js renamed to test/integration/js/shared/test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,3 @@ function exit() {
8383

8484
if (!success || only || ignored) { process.exit(1) } else { process.exit(0) }
8585
}
86-

0 commit comments

Comments
 (0)