Skip to content
This repository was archived by the owner on Sep 7, 2022. It is now read-only.

fix child process hanging on unclosed connections #46

Open
jkruse14 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix child process hanging on unclosed connections #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a2b34a
fix child process hanging on unclosed connections
Mar 6, 2020
9bfc7b1
refactor and test fix ups
Mar 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 15 additions & 19 deletions lib/scheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const schedule = require("node-schedule");
const utils = require("./utils");
const childProcess = require("child_process");
const path = require('path');

const DEFAULT_TIMEOUT = 6;
const MS_PER_SEC = 1000;
Expand Down Expand Up @@ -81,27 +81,24 @@ class Scheduler {
this.serverless.cli.log(`scheduler: scheduling ${fConfig.id}/${eventData.name} `
+ `with ${eventData.cron}`);
this.serverless.cli.log(`${eventData.name}`);
schedule.scheduleJob(eventData.cron, () => {
this._executeFunction(fConfig.id, eventData.input);
});
schedule.scheduleJob(eventData.cron, () => this._executeFunction(fConfig.id, fConfig.modulePath, fConfig.handlerFunction, fConfig.timeout, eventData.input));

if (options.runSchedulesOnInit) {
this._executeFunction(fConfig.id, eventData.input);
return this._executeFunction(fConfig.id, fConfig.modulePath, fConfig.handlerFunction, fConfig.timeout, eventData.input);
}
}
}
return Promise.resolve();
}

_executeFunction(fName, fInput) {
const args = [process.argv[1], "invoke", "local", "--function", fName]
if (fInput) {
args.push("--data", JSON.stringify(fInput));
}
for (const { name, value } of this._getSlsInvokeOptions()) {
args.push(`--${name}`, value);
}
return childProcess.execFileSync(process.argv[0], args, { cwd: "./", stdio: "inherit" });
_buildModulePath(configPath) {
// need to back out of the node_modules folder
return path.join(__dirname, `../../../${configPath}`);
}

_executeFunction(functionId, handlerPath, handlerFunctionName, functionTimeout, fInput) {
const modulePath = this._buildModulePath(handlerPath);
return require(modulePath)[handlerFunctionName](fInput, this._getContext(functionId, functionTimeout));
}

_getSlsInvokeOptions() {
Expand All @@ -128,10 +125,8 @@ class Scheduler {
Object.assign(process.env, baseEnv, providerEnvVars, functionEnvVars);
}

_getContext(fConfig) {
const functionName = fConfig.id;

const timeout = fConfig.timeout || this.serverless.service.provider.timeout || DEFAULT_TIMEOUT;
_getContext(functionName, timeout) {
timeout = timeout || this.serverless.service.provider.timeout || DEFAULT_TIMEOUT;

const endTime = Math.max(0, Date.now() + timeout * MS_PER_SEC);

Expand Down Expand Up @@ -214,7 +209,8 @@ class Scheduler {
id: funcName,
events: scheduleEvents,
timeout: funcConf.timeout,
moduleName: funcConf.handler.split(".")[0]
moduleName: funcConf.handler.split(".")[0],
handlerFunction: funcConf.handler.split(".")[1]
});
}
}
Expand Down
95 changes: 47 additions & 48 deletions tests/scheduler.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
"use strict";
/* eslint-env mocha */

const childProcess = require("child_process");
const testHandler = require('./test-handler')
const Serverless = require("serverless");
const Scheduler = require("../lib/scheduler");

const MS_PER_SEC = 1000;

jest.mock("child_process");
jest.mock('./test-handler');

describe("validate", () => {
let module;
Expand Down Expand Up @@ -212,22 +211,23 @@ describe("validate", () => {
expect(funcs[0].events).toHaveLength(1);

const event = funcs[0].events[0];
module._executeFunction(funcs[0].id, event.input);
module._buildModulePath = () => '../tests/test-handler';
module._executeFunction(funcs[0].id, funcs[0].moduleName, funcs[0].handlerFunction, funcs[0].timeout, event.input);

expect(event.cron).toEqual("1/* * * * *");
expect(event.input.key1).toEqual("value1");
expect(childProcess.execFileSync).toBeCalledWith(
process.argv[0],
[
process.argv[1],
"invoke",
"local",
"--function",
funcs[0].id,
"--data",
JSON.stringify(event.input),
],
{cwd: "./", stdio: "inherit" }
expect(testHandler.test1).toBeCalledWith(
event.input,
expect.objectContaining({
callbackWaitsForEmptyEventLoop: true,
functionName: funcs[0].id,
functionVersion: "$LATEST",
invokedFunctionArn: `arn:aws:lambda:serverless-offline:123456789012:function:${funcs[0].id}`,
isDefaultFunctionVersion: true,
logGroupName: `/aws/lambda/${funcs[0].id}`,
logStreamName: expect.any(String),
memoryLimitInMB: "1024",
})
);
});

Expand Down Expand Up @@ -258,24 +258,23 @@ describe("validate", () => {
expect(funcs[0].events).toHaveLength(1);

const event = funcs[0].events[0];
module._executeFunction(funcs[0].id, event.input);
module._buildModulePath = () => '../tests/test-handler';
module._executeFunction(funcs[0].id, funcs[0].moduleName, funcs[0].handlerFunction, funcs[0].timeout, event.input);

expect(event.cron).toEqual("1/* * * * *");
expect(event.input.key1).toEqual("value1");
expect(childProcess.execFileSync).toBeCalledWith(
process.argv[0],
[
process.argv[1],
"invoke",
"local",
"--function",
funcs[0].id,
"--data",
JSON.stringify(event.input),
"--option1",
"value2"
],
{cwd: "./", stdio: "inherit" }
expect(testHandler.test1).toBeCalledWith(
event.input,
expect.objectContaining({
callbackWaitsForEmptyEventLoop: true,
functionName: funcs[0].id,
functionVersion: "$LATEST",
invokedFunctionArn: `arn:aws:lambda:serverless-offline:123456789012:function:${funcs[0].id}`,
isDefaultFunctionVersion: true,
logGroupName: `/aws/lambda/${funcs[0].id}`,
logStreamName: expect.any(String),
memoryLimitInMB: "1024",
})
);
});

Expand All @@ -300,21 +299,21 @@ describe("validate", () => {
expect(funcs[0].events).toHaveLength(1);

const event = funcs[0].events[0];
module._executeFunction(funcs[0].id, event.input);
module._buildModulePath = () => '../tests/test-handler';
module._executeFunction(funcs[0].id, funcs[0].moduleName, funcs[0].handlerFunction, funcs[0].timeout, event.input);

expect(event.cron).toEqual("1/* * * * *");
expect(childProcess.execFileSync).toBeCalledWith(
process.argv[0],
[
process.argv[1],
"invoke",
"local",
"--function",
funcs[0].id,
"--data",
JSON.stringify(event.input)
],
{ cwd: "./", stdio: "inherit" }
expect(testHandler.test1).toBeCalledWith(
event.input, expect.objectContaining({
callbackWaitsForEmptyEventLoop: true,
functionName: "scheduled1",
functionVersion: "$LATEST",
invokedFunctionArn: "arn:aws:lambda:serverless-offline:123456789012:function:scheduled1",
isDefaultFunctionVersion: true,
logGroupName: "/aws/lambda/scheduled1",
logStreamName: expect.any(String),
memoryLimitInMB: "1024",
})
);
});

Expand All @@ -337,7 +336,7 @@ describe("validate", () => {
};

const funcs = module._getFuncConfigs();
const context = module._getContext(funcs[0]);
const context = module._getContext(funcs[0].id, funcs[0].timeout);
expect(context.getRemainingTimeInMillis()).toBeLessThanOrEqual(timeout * MS_PER_SEC);
expect(context.getRemainingTimeInMillis()).toBeGreaterThan(timeout * MS_PER_SEC - maxDuration);
});
Expand All @@ -361,14 +360,14 @@ describe("validate", () => {
};

const funcs = module._getFuncConfigs();
const context = module._getContext(funcs[0]);
const context = module._getContext(funcs[0].id, funcs[0].timeout);
expect(context.getRemainingTimeInMillis()).toBeLessThanOrEqual(timeout * MS_PER_SEC);
expect(context.getRemainingTimeInMillis()).toBeGreaterThan(timeout * MS_PER_SEC - maxDuration);
});

it("should use the *default* timeout for getRemainingTimeInMillis", () => {
const timeout = 6; // secs
const maxDuration = 2; // msecs
const maxDuration = 2.1; // msecs

module.serverless.service.functions = {
scheduled1: {
Expand All @@ -384,7 +383,7 @@ describe("validate", () => {
};

const funcs = module._getFuncConfigs();
const context = module._getContext(funcs[0]);
const context = module._getContext(funcs[0].id, funcs[0].timeout);
expect(context.getRemainingTimeInMillis()).toBeLessThanOrEqual(timeout * MS_PER_SEC);
expect(context.getRemainingTimeInMillis()).toBeGreaterThan(timeout * MS_PER_SEC - maxDuration);
});
Expand Down
1 change: 1 addition & 0 deletions tests/test-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports.test1 = () => Promise.resolve();