Skip to content

Commit 1299a33

Browse files
vasil-cfvasil-cf
authored
fix: telemetry termination (#153)
1 parent d084eca commit 1299a33

File tree

9 files changed

+109
-103
lines changed

9 files changed

+109
-103
lines changed

‎lib/addNewMask.js‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/* eslint-disable promise/catch-or-return */
22

33
// ↓ Should be imported first
4-
const { terminate } = require('@codefresh-io/cf-telemetry/init');
4+
require('@codefresh-io/cf-telemetry/init');
55
// ↓ Keep one blank line below to prevent automatic import reordering
66

77
const { Logger } = require('@codefresh-io/cf-telemetry/logs');
8-
const { getServerAddress, registerExitHandlers } = require('./helpers');
8+
const { getServerAddress, registerExitHandlers, shutdownGracefully } = require('./helpers');
99

1010
const logger = new Logger('codefresh:containerLogger:addNewMask');
1111

@@ -45,14 +45,14 @@ async function updateMasks(secret) {
4545
if (response.statusCode === 201) {
4646
logger.log(`successfully updated masks with secret: ${secret.key}`);
4747
exitWithError = false;
48-
terminate().finally(() => process.exit(exitCodes.success));
48+
await shutdownGracefully(exitCodes.success);
4949
} else {
5050
logger.error(`could not create mask for secret: ${secret.key}. Server responded with: ${response.statusCode}\n\n${response.body}`);
51-
terminate().finally(() => process.exit(exitCodes.error));
51+
await shutdownGracefully(exitCodes.error);
5252
}
5353
} catch (error) {
5454
logger.error(`could not create mask for secret: ${secret.key}. Error: ${error}`);
55-
terminate().finally(() => process.exit(exitCodes.error));
55+
await shutdownGracefully(exitCodes.error);
5656
}
5757
}
5858

@@ -62,11 +62,12 @@ if (require.main === module) {
6262
// first argument is the secret key second argument is the secret value
6363
if (process.argv.length < 4) {
6464
logger.log('not enough arguments, need secret key and secret value');
65-
terminate().finally(() => process.exit(exitCodes.missingArguments));
65+
shutdownGracefully(exitCodes.missingArguments);
66+
} else {
67+
const key = process.argv[2];
68+
const value = process.argv[3];
69+
updateMasks({ key, value });
6670
}
67-
const key = process.argv[2];
68-
const value = process.argv[3];
69-
updateMasks({ key, value });
7071
} else {
7172
module.exports = { updateMasks, exitHandler };
7273
}

‎lib/helpers.js‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,33 @@ const getServerAddress = async () => {
6060
}
6161
};
6262

63+
const shutdownGracefully = async (code) => {
64+
try {
65+
await terminate();
66+
} catch (error) {
67+
// supress any error during terminating telemetry
68+
logger.error(`Error occurred while terminating telemetry.`, error);
69+
}
70+
71+
process.exit(code);
72+
};
73+
6374
/**
6475
* As `@codefresh-io/cf-telemetry/init` changes the original node.js behavior of how SIGTERM and SIGINT
6576
* signals are handled, we revert this change back to the original node.js behavior.
6677
*/
6778
const registerExitHandlers = () => {
68-
process.on('SIGTERM', () => {
69-
terminate().finally(() => process.exit(143)); // default exit code for SIGTERM
79+
process.on('SIGTERM', async () => {
80+
await shutdownGracefully(143); // default exit code for SIGTERM
7081
});
7182

72-
process.on('SIGINT', () => {
73-
terminate().finally(() => process.exit(130)); // default exit code for SIGINT
83+
process.on('SIGINT', async () => {
84+
await shutdownGracefully(130); // default exit code for SIGINT
7485
});
7586
};
7687

7788
module.exports = {
89+
shutdownGracefully,
7890
watchForBuildFinishedSignal,
7991
saveServerAddress,
8092
getServerAddress,

‎lib/index.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ const logger = new Logger({
3232
showProgress: process.env.SHOW_PROGRESS === 'true',
3333
});
3434

35-
logger.validate();
36-
logger.start();
35+
// eslint-disable-next-line promise/catch-or-return
36+
logger.validate()
37+
.then(() => logger.start());
3738

3839
process.on('beforeExit', (code) => {
3940
logs.log(`beforeExit: ${code}`);

‎lib/isReady.js‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
/* eslint-disable promise/catch-or-return */
22

33
// ↓ Should be imported first
4-
const { terminate } = require('@codefresh-io/cf-telemetry/init');
4+
require('@codefresh-io/cf-telemetry/init');
55
// ↓ Keep one blank line below to prevent automatic import reordering
66

77
const { readFileSync } = require('fs');
88
const _ = require('lodash');
99
const { Logger } = require('@codefresh-io/cf-telemetry/logs');
1010
const { ContainerHandlingStatus } = require('./enums');
11-
const { registerExitHandlers } = require('./helpers');
11+
const { registerExitHandlers, shutdownGracefully } = require('./helpers');
1212

1313
registerExitHandlers();
1414

@@ -33,15 +33,21 @@ function isContainerLoggerReady(state) {
3333
return isReady;
3434
}
3535

36-
(() => {
36+
const isReady = async () => {
3737
const containerId = process.argv[2];
3838
const state = JSON.parse(readFileSync('./lib/state.json').toString('utf-8'));
39-
let isReady = false;
39+
let ready = false;
4040
if (containerId) {
41-
isReady = isContainerReady(state, containerId);
41+
ready = isContainerReady(state, containerId);
4242
} else {
43-
isReady = isContainerLoggerReady(state);
43+
ready = isContainerLoggerReady(state);
4444
}
4545

46-
terminate().finally(() => process.exit(isReady ? 0 : 1));
47-
})();
46+
await shutdownGracefully(ready ? 0 : 1);
47+
};
48+
49+
if (require.main === module) {
50+
isReady();
51+
} else {
52+
module.exports = { isReady };
53+
}

‎lib/logger.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable promise/catch-or-return */
22

33
// ↓ Should be imported first
4-
const { terminate } = require('@codefresh-io/cf-telemetry/init');
4+
require('@codefresh-io/cf-telemetry/init');
55
// ↓ Keep one blank line below to prevent automatic import reordering
66

77
const fs = require('fs');
@@ -19,6 +19,7 @@ const ContainerLogger = require('./ContainerLogger');
1919

2020
// eslint-disable-next-line import/no-unresolved,import/extensions
2121
const { HttpServer } = require('./http-server');
22+
const { shutdownGracefully } = require('./helpers');
2223

2324
const logger = new Logs('codefresh:containerLogger');
2425

@@ -70,12 +71,12 @@ class Logger {
7071
* validates the passed params of the constructor
7172
* @returns {*}
7273
*/
73-
validate() {
74+
async validate() {
7475
if (!this.taskLoggerConfig) {
75-
return this._error(new CFError('taskLogger configuration is missing'));
76+
return await this._error(new CFError('taskLogger configuration is missing'));
7677
}
7778
if (!this.loggerId) {
78-
return this._error(new CFError('logger id is missing'));
79+
return await this._error(new CFError('logger id is missing'));
7980
}
8081
return undefined;
8182
}
@@ -126,12 +127,11 @@ class Logger {
126127
this._listenForExistingContainers();
127128
}
128129
})
129-
.catch((err) => {
130-
this._error(new CFError({
130+
.catch(async (err) => {
131+
await this._error(new CFError({
131132
cause: err,
132133
message: `Failed to create taskLogger`
133134
}));
134-
135135
});
136136

137137
await this._listenForEngineRequests();
@@ -155,9 +155,9 @@ class Logger {
155155
* will print the error and exit the process
156156
* @param err
157157
*/
158-
_error(err) {
158+
async _error(err) {
159159
logger.error(err.toString());
160-
terminate().finally(() => process.exit(1));
160+
await shutdownGracefully(1);
161161
}
162162

163163
/**

‎service.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
version: 1.13.0
1+
version: 1.13.1

‎test/addNewMask.unit.spec.js‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const chai = require('chai');
22
const sinon = require('sinon');
33
const sinonChai = require('sinon-chai');
4+
const {shutdownGracefully} = require("../lib/helpers");
45
const proxyquire = require('proxyquire').noCallThru();
56

67
const expect = chai.expect;
@@ -57,12 +58,10 @@ describe('addNewMask', () => {
5758
stubGot.post.resolves({ statusCode: 201 });
5859

5960
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
60-
'@codefresh-io/cf-telemetry/init': {
61-
terminate: () => ({
62-
finally: callback => callback(),
63-
})
61+
'./helpers': {
62+
getServerAddress: stubGetServerAddress,
63+
shutdownGracefully,
6464
},
65-
'./helpers': { getServerAddress: stubGetServerAddress },
6665
});
6766
process.listeners('exit').forEach((listener) => {
6867
if (listener === exitHandler) {
@@ -82,16 +81,12 @@ describe('addNewMask', () => {
8281
it('should fail if the server address is not available', async () => {
8382
stubGetServerAddress.rejects('could not get server address');
8483
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
85-
'@codefresh-io/cf-telemetry/init': {
86-
terminate: () => ({
87-
finally: callback => callback(),
88-
})
89-
},
9084
'@codefresh-io/cf-telemetry/logs': {
9185
Logger: function() { return stubLogger },
9286
},
9387
'./helpers': {
9488
getServerAddress: stubGetServerAddress,
89+
shutdownGracefully,
9590
},
9691
});
9792
process.listeners('exit').forEach((listener) => {
@@ -107,16 +102,12 @@ describe('addNewMask', () => {
107102
it('should fail if the server address is not valid URL', async () => {
108103
stubGetServerAddress.resolves('foo');
109104
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
110-
'@codefresh-io/cf-telemetry/init': {
111-
terminate: () => ({
112-
finally: callback => callback(),
113-
})
114-
},
115105
'@codefresh-io/cf-telemetry/logs': {
116106
Logger: function() { return stubLogger },
117107
},
118108
'./helpers': {
119109
getServerAddress: stubGetServerAddress,
110+
shutdownGracefully,
120111
},
121112
});
122113
process.listeners('exit').forEach((listener) => {
@@ -137,15 +128,13 @@ describe('addNewMask', () => {
137128
body: 'Internal Server Error',
138129
});
139130
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
140-
'@codefresh-io/cf-telemetry/init': {
141-
terminate: () => ({
142-
finally: callback => callback(),
143-
})
144-
},
145131
'@codefresh-io/cf-telemetry/logs': {
146132
Logger: function() { return stubLogger },
147133
},
148-
'./helpers': { getServerAddress: stubGetServerAddress },
134+
'./helpers': {
135+
getServerAddress: stubGetServerAddress,
136+
shutdownGracefully,
137+
},
149138
});
150139
process.listeners('exit').forEach((listener) => {
151140
if (listener === exitHandler) {
@@ -202,7 +191,10 @@ describe('addNewMask', () => {
202191
stubGetServerAddress.resolves(serverAddress);
203192
stubGot.post.resolves({ statusCode: 201 });
204193
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
205-
'./helpers': { getServerAddress: stubGetServerAddress },
194+
'./helpers': {
195+
getServerAddress: stubGetServerAddress,
196+
shutdownGracefully,
197+
},
206198
});
207199
process.listeners('exit').forEach((listener) => {
208200
if (listener === exitHandler) {

0 commit comments

Comments
 (0)