Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Commit

Permalink
fix: close http connections after http-server.close() has been called (
Browse files Browse the repository at this point in the history
…#2667)

fixes #2185

Co-authored-by: David Murdoch <[email protected]>
Co-authored-by: Micaiah Reid <[email protected]>
  • Loading branch information
3 people committed Apr 4, 2022
1 parent eeab16a commit 3086e7f
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 67 deletions.
6 changes: 3 additions & 3 deletions src/chains/ethereum/ethereum/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/chains/ethereum/ethereum/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"ws": "8.2.3"
},
"devDependencies": {
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.3",
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.4",
"@types/encoding-down": "5.0.0",
"@types/fs-extra": "9.0.2",
"@types/keccak": "3.0.1",
Expand Down
6 changes: 3 additions & 3 deletions src/chains/filecoin/filecoin/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/chains/filecoin/filecoin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"@filecoin-shipyard/lotus-client-schema": "2.0.0",
"@ganache/filecoin-options": "0.1.4",
"@ganache/utils": "0.1.4",
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.3",
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.4",
"@types/bn.js": "5.1.0",
"@types/deep-equal": "1.0.1",
"@types/levelup": "4.3.0",
Expand Down
6 changes: 3 additions & 3 deletions src/chains/tezos/tezos/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/chains/tezos/tezos/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"emittery": "0.10.0"
},
"devDependencies": {
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.3",
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.4",
"@types/mocha": "9.0.0",
"cheerio": "1.0.0-rc.3",
"cross-env": "7.0.3",
Expand Down
6 changes: 3 additions & 3 deletions src/packages/core/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"@ganache/options": "0.1.4",
"@ganache/tezos": "0.1.4",
"@ganache/utils": "0.1.4",
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.3",
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.4",
"aggregate-error": "3.1.0",
"emittery": "0.10.0",
"promise.allsettled": "1.0.4"
Expand Down
24 changes: 20 additions & 4 deletions src/packages/core/src/servers/http-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ function prepareCORSResponseHeaders(method: HttpMethods, request: HttpRequest) {

function sendResponse(
response: HttpResponse,
closeConnection: boolean,
statusCode: HttpResponseCodes,
contentType: RecognizedString | null,
data: RecognizedString | null,
Expand All @@ -82,10 +83,12 @@ function sendResponse(
if (contentType != null) {
response.writeHeader("Content-Type", contentType);
}
if (data != null) {
response.end(data);

if (data !== null) {
response.end(data, closeConnection);
} else {
response.end();
// in the case that body is not provided, it must specifically be <undefined> and not <null>
response.end(undefined, closeConnection);
}
});
}
Expand All @@ -94,6 +97,8 @@ export type HttpServerOptions = Pick<InternalOptions["server"], "rpcEndpoint">;

export default class HttpServer {
#connector: Connector;
#isClosing = false;

constructor(
app: TemplatedApp,
connector: Connector,
Expand All @@ -110,6 +115,7 @@ export default class HttpServer {
app.get("/418", response => {
sendResponse(
response,
this.#isClosing,
HttpResponseCodes.IM_A_TEAPOT,
ContentTypes.PLAIN,
"418 I'm a teapot"
Expand All @@ -124,6 +130,7 @@ export default class HttpServer {
// a client tried to connect via websocket. This is a Bad Request.
sendResponse(
response,
this.#isClosing,
HttpResponseCodes.BAD_REQUEST,
ContentTypes.PLAIN,
"400 Bad Request"
Expand All @@ -132,6 +139,7 @@ export default class HttpServer {
// all other requests don't mean anything to us, so respond with `404 Not Found`...
sendResponse(
response,
this.#isClosing,
HttpResponseCodes.NOT_FOUND,
ContentTypes.PLAIN,
"404 Not Found"
Expand Down Expand Up @@ -167,6 +175,7 @@ export default class HttpServer {
} catch (e: any) {
sendResponse(
response,
this.#isClosing,
HttpResponseCodes.BAD_REQUEST,
ContentTypes.PLAIN,
"400 Bad Request: " + e.message,
Expand Down Expand Up @@ -199,6 +208,7 @@ export default class HttpServer {
} else {
sendResponse(
response,
this.#isClosing,
HttpResponseCodes.OK,
ContentTypes.JSON,
data,
Expand All @@ -215,6 +225,7 @@ export default class HttpServer {
const data = connector.formatError(error, payload);
sendResponse(
response,
this.#isClosing,
HttpResponseCodes.OK,
ContentTypes.JSON,
data,
Expand All @@ -237,13 +248,18 @@ export default class HttpServer {
// OPTIONS responses don't have a body, so respond with `204 No Content`...
sendResponse(
response,
this.#isClosing,
HttpResponseCodes.NO_CONTENT,
null,
null,
writeHeaders
);
};

public close() {
// currently a no op.
// flags the server as closing, indicating the connection should be closed with subsequent responses
// as there is no way presently to close existing connections outside of the request/response context
// see discussion: https://github.com/uNetworking/uWebSockets.js/issues/663#issuecomment-1026283415
this.#isClosing = true;
}
}
140 changes: 97 additions & 43 deletions src/packages/core/tests/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,53 +63,55 @@ describe("server", () => {
}
}

/**
* Sends a post request to the server and returns the response.
* @param address
* @param port
* @param json
* @param agent
* @returns
*/
function post(host: string, port: number, json: any, agent?: any) {
const data = JSON.stringify(json);
// We use http.request instead of superagent because superagent doesn't
// support the interface name in ipv6 addresses, and in GitHub Actions the
// Mac tests would fail because one of the available ipv6 addresses
// requires the interface name (`fe80::1%lo0`)
const req = http.request({
agent,
method: "POST",
protocol: "http:",
host,
port,
headers: {
accept: "application/json",
"content-type": "application/json",
"content-length": Buffer.byteLength(data)
}
});
let resolve: any;
let reject: any;
const deferred = new Promise<any>((_resolve, _reject) => {
resolve = _resolve;
reject = _reject;
});
req.on("response", (res: http.IncomingMessage) => {
let data = "";
res
.on("data", d => data += d.toString("utf8"))
.on("end", () => resolve({ status: 200, body: JSON.parse(data) }));
});
req.on("error", (err) => reject(err));
req.write(data);
req.end();
return deferred;
}

// skip this test unless in GitHub Actions, as this test iterates over
// all available network interfacesnand network interfaces on user
// all available network interfaces and network interfaces on user
// machines are unpredictible and may behave in ways that we don't care
// about.
(process.env.GITHUB_ACTION ? describe : describe.skip)("listen", function () {
/**
* Sends a post request to the server and returns the response.
* @param address
* @param port
* @param json
* @returns
*/
function post(host: string, port: number, json: any) {
const data = JSON.stringify(json);
// We use http.request instead of superagent because superagent doesn't
// support the interface name in ipv6 addresses, and in GitHub Actions the
// Mac tests would fail because one of the available ipv6 addresses
// requires the interface name (`fe80::1%lo0`)
const req = http.request({
method: "POST",
protocol: "http:",
host,
port,
headers: {
accept: "application/json",
"content-type": "application/json",
"content-length": Buffer.byteLength(data)
}
});
let resolve: any;
let reject: any;
const deferred = new Promise<any>((_resolve, _reject) => {
resolve = _resolve;
reject = _reject;
});
req.on("response", (res: http.IncomingMessage) => {
let data = "";
res
.on("data", d => data += d.toString("utf8"))
.on("end", () => resolve({ status: 200, body: JSON.parse(data) }));
});
req.on("error", (err) => reject(err));
req.write(data);
req.end();
return deferred;
}

function getHost(info: NetworkInterfaceInfo, interfaceName: string) {
// a link-local ipv6 address starts with fe80:: and _must_ include a "zone_id"
if (info.family === "IPv6" && info.address.startsWith("fe80::")) {
Expand Down Expand Up @@ -391,6 +393,58 @@ describe("server", () => {
}
});

it("closes even if a persistent http connection is open", async () => {
const agent = new http.Agent({
keepAlive: true
});

await setup();

try {
// open the http connection
await post("localhost", port, jsonRpcJson, agent);

await s.close();
// a request is required in order to actually close the connection
// see https://github.com/trufflesuite/ganache/issues/2788
await post("localhost", port, jsonRpcJson, agent);

// connection has now closed, allowing ganache to close
await assert.rejects(post("localhost", port, jsonRpcJson, agent), {
code: "ECONNREFUSED"
});
} finally {
teardown();
}
});

it("refuses new connections when waiting on persistent connections to close", async () => {
const agent = new http.Agent({
keepAlive: true
});

await setup();

try {
// open the http connection
await post("localhost", port, jsonRpcJson, agent);

await s.close();

// this connection is on a different connection, so should fail
await assert.rejects(post("localhost", port, jsonRpcJson), {
code: "ECONNREFUSED"
});

// a request is required in order to actually close the connection
// see https://github.com/trufflesuite/ganache/issues/2788
await post("localhost", port, jsonRpcJson, agent);

} finally {
teardown();
}
});

it("fails to listen if the socket is already in use by 3rd party, Promise", async () => {
const server = http.createServer();
server.listen(port);
Expand Down
6 changes: 3 additions & 3 deletions src/packages/utils/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/packages/utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"seedrandom": "3.0.5"
},
"devDependencies": {
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.3",
"@trufflesuite/uws-js-unofficial": "20.4.0-unofficial.4",
"@types/mocha": "9.0.0",
"@types/seedrandom": "3.0.1",
"cross-env": "7.0.3",
Expand Down

0 comments on commit 3086e7f

Please sign in to comment.