Skip to content

Commit cd16e01

Browse files
vasil-cfvasil-cf
committed
fix: telemetry termination
1 parent d084eca commit cd16e01

File tree

8 files changed

+108
-92
lines changed

8 files changed

+108
-92
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: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -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
/**

‎test/addNewMask.unit.spec.js‎

Lines changed: 15 additions & 13 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;
@@ -62,7 +63,10 @@ describe('addNewMask', () => {
6263
finally: callback => callback(),
6364
})
6465
},
65-
'./helpers': { getServerAddress: stubGetServerAddress },
66+
'./helpers': {
67+
getServerAddress: stubGetServerAddress,
68+
shutdownGracefully,
69+
},
6670
});
6771
process.listeners('exit').forEach((listener) => {
6872
if (listener === exitHandler) {
@@ -82,16 +86,12 @@ describe('addNewMask', () => {
8286
it('should fail if the server address is not available', async () => {
8387
stubGetServerAddress.rejects('could not get server address');
8488
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
85-
'@codefresh-io/cf-telemetry/init': {
86-
terminate: () => ({
87-
finally: callback => callback(),
88-
})
89-
},
9089
'@codefresh-io/cf-telemetry/logs': {
9190
Logger: function() { return stubLogger },
9291
},
9392
'./helpers': {
9493
getServerAddress: stubGetServerAddress,
94+
shutdownGracefully,
9595
},
9696
});
9797
process.listeners('exit').forEach((listener) => {
@@ -107,16 +107,12 @@ describe('addNewMask', () => {
107107
it('should fail if the server address is not valid URL', async () => {
108108
stubGetServerAddress.resolves('foo');
109109
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
110-
'@codefresh-io/cf-telemetry/init': {
111-
terminate: () => ({
112-
finally: callback => callback(),
113-
})
114-
},
115110
'@codefresh-io/cf-telemetry/logs': {
116111
Logger: function() { return stubLogger },
117112
},
118113
'./helpers': {
119114
getServerAddress: stubGetServerAddress,
115+
shutdownGracefully,
120116
},
121117
});
122118
process.listeners('exit').forEach((listener) => {
@@ -145,7 +141,10 @@ describe('addNewMask', () => {
145141
'@codefresh-io/cf-telemetry/logs': {
146142
Logger: function() { return stubLogger },
147143
},
148-
'./helpers': { getServerAddress: stubGetServerAddress },
144+
'./helpers': {
145+
getServerAddress: stubGetServerAddress,
146+
shutdownGracefully,
147+
},
149148
});
150149
process.listeners('exit').forEach((listener) => {
151150
if (listener === exitHandler) {
@@ -202,7 +201,10 @@ describe('addNewMask', () => {
202201
stubGetServerAddress.resolves(serverAddress);
203202
stubGot.post.resolves({ statusCode: 201 });
204203
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
205-
'./helpers': { getServerAddress: stubGetServerAddress },
204+
'./helpers': {
205+
getServerAddress: stubGetServerAddress,
206+
shutdownGracefully,
207+
},
206208
});
207209
process.listeners('exit').forEach((listener) => {
208210
if (listener === exitHandler) {

0 commit comments

Comments
 (0)