Skip to content

Commit

Permalink
Resolve tests timeout and closes connections
Browse files Browse the repository at this point in the history
This is related to nodejs/node#2642
Added stoppable module and stops the server.
Also added --exit to the tables test, so it does not hang.
Both conditions were holding the test process indefinitely.
  • Loading branch information
giggio committed Nov 17, 2022
1 parent 2453337 commit b95a2e3
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 12 deletions.
8 changes: 8 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
## Upcoming Release

General:

- Stop accepting new connections and closes existing, idle connections (including keep-alives) without killing requests that are in-flight.

Table:

- Added exit parameter to tests so they don't hang.

## 2022.10 Version 3.20.1

General:
Expand Down
20 changes: 20 additions & 0 deletions package-lock.json

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

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"mysql2": "^2.1.0",
"rimraf": "^3.0.2",
"sequelize": "^6.3.0",
"stoppable": "^1.1.0",
"tedious": "^15.0.0",
"to-readable-stream": "^2.1.0",
"tslib": "^2.3.0",
Expand Down Expand Up @@ -60,6 +61,7 @@
"@types/multistream": "^2.1.1",
"@types/node": "^14.14.24",
"@types/rimraf": "^3.0.0",
"@types/stoppable": "^1.1.1",
"@types/uri-templates": "^0.1.29",
"@types/uuid": "^3.4.4",
"@types/validator": "^13.1.4",
Expand Down Expand Up @@ -285,7 +287,7 @@
"test:blob:sql": "npm run lint && cross-env cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 AZURITE_TEST_DB=mysql://root:[email protected]:3306/azurite_blob_test mocha --compilers ts-node/register --no-timeouts --grep @sql --recursive tests/blob/*.test.ts tests/blob/**/*.test.ts",
"test:blob:sql:ci": "npm run lint && cross-env cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 AZURITE_TEST_DB=mysql://root:[email protected]:13306/azurite_blob_test mocha --compilers ts-node/register --no-timeouts --grep @sql --recursive tests/blob/*.test.ts tests/blob/**/*.test.ts",
"test:queue": "npm run lint && cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 mocha --compilers ts-node/register --no-timeouts --recursive tests/queue/*.test.ts tests/queue/**/*.test.ts",
"test:table": "npm run lint && cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 mocha --compilers ts-node/register --no-timeouts --recursive tests/table/**/*.test.ts",
"test:table": "npm run lint && cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 mocha --compilers ts-node/register --no-timeouts --recursive --exit tests/table/**/*.test.ts",
"test:exe": "npm run lint && cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 mocha --compilers ts-node/register --no-timeouts tests/exe.test.ts --exit",
"test:linux": "npm run lint && cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 mocha --compilers ts-node/register --no-timeouts tests/linuxbinary.test.ts --exit",
"clean": "rimraf dist typings *.log coverage __testspersistence__ temp __testsstorage__ .nyc_output debug.log *.vsix *.tgz",
Expand Down
16 changes: 5 additions & 11 deletions src/common/ServerBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as https from "https";
import ConfigurationBase from "./ConfigurationBase";
import ICleaner from "./ICleaner";
import IRequestListenerFactory from "./IRequestListenerFactory";
import stoppable from "stoppable";

export type RequestListener = (
request: http.IncomingMessage,
Expand All @@ -26,6 +27,7 @@ export enum ServerStatus {
*/
export default abstract class ServerBase implements ICleaner {
protected status: ServerStatus = ServerStatus.Closed;
public readonly httpServer: (http.Server | https.Server) & stoppable.WithStop;

/**
* Creates an instance of HTTP or HTTPS server.
Expand All @@ -39,11 +41,12 @@ export default abstract class ServerBase implements ICleaner {
public constructor(
public readonly host: string,
public readonly port: number,
public readonly httpServer: http.Server | https.Server,
httpServer: http.Server | https.Server,
requestListenerFactory: IRequestListenerFactory,
public readonly config: ConfigurationBase
) {
// Remove predefined request listeners to avoid double request handling
this.httpServer = stoppable(httpServer);
this.httpServer.removeAllListeners("request");
this.httpServer.on(
"request",
Expand Down Expand Up @@ -133,16 +136,7 @@ export default abstract class ServerBase implements ICleaner {
// Remove request listener to reject incoming requests
this.httpServer.removeAllListeners("request");

// Close HTTP server first to deny incoming connections
// You will find this will not close server immediately because there maybe existing keep-alive connections
// Calling httpServer.close will only stop accepting incoming requests
// and wait for existing keep-alive connections timeout
// Default keep-alive timeout is 5 seconds defined by httpServer.keepAliveTimeout
// TODO: Add a middleware to reject incoming request over existing keep-alive connections
// https://github.com/nodejs/node/issues/2642
await new Promise(resolve => {
this.httpServer.close(resolve);
});
this.httpServer.stop();

await this.afterClose();

Expand Down

0 comments on commit b95a2e3

Please sign in to comment.