refactor(Telemetry): Ensure generation and related utils are sync

This commit is contained in:
Piotr Grzesik 2021-07-05 17:42:23 +02:00
parent 9b624a5067
commit e65199c052
18 changed files with 144 additions and 143 deletions

View File

@ -226,7 +226,7 @@ class Serverless {
);
}
if (this.isLocallyInstalled) return;
const localServerlessPath = await resolveLocalServerlessPath();
const localServerlessPath = resolveLocalServerlessPath();
if (!localServerlessPath) return;
if (localServerlessPath === serverlessPath) {
this.isLocallyInstalled = true;

View File

@ -615,8 +615,8 @@ class PluginManager {
// TODO: Remove this logic with `v3.0.0` along with removal of `isTelemetryReportedExternally`
// After we are ensured that local fallback from `v3` will always fall back to `v3` local installation
if (!this.serverless.isTelemetryReportedExternally && !isTelemetryDisabled) {
await storeTelemetryLocally(
await generateTelemetryPayload({
storeTelemetryLocally(
generateTelemetryPayload({
...resolveCliInput(),
serviceDir: this.serverless.serviceDir,
configuration: this.serverless.configurationInput,

View File

@ -61,8 +61,8 @@ module.exports = async (exception, options = {}) => {
// If provided serverless instance is a local fallback, and we're not in context of it
// Pass error handling to this local fallback implementation
if (isInvokedByGlobalInstallation && !(await resolveIsLocallyInstalled())) {
const localServerlessPath = await resolveLocalServerlessPath();
if (isInvokedByGlobalInstallation && !resolveIsLocallyInstalled()) {
const localServerlessPath = resolveLocalServerlessPath();
try {
// Attempt to use error handler from local version
@ -136,7 +136,7 @@ module.exports = async (exception, options = {}) => {
const installationModePostfix = await (async () => {
if (isStandaloneExecutable) return ' (standalone)';
if (isLocallyInstalled != null) return isLocallyInstalled ? ' (local)' : '';
return (await resolveIsLocallyInstalled()) ? ' (local)' : '';
return resolveIsLocallyInstalled() ? ' (local)' : '';
})();
consoleLog(
chalk.yellow(` Framework Version: ${slsVersion}${installationModePostfix}`)
@ -178,7 +178,7 @@ module.exports = async (exception, options = {}) => {
if (commandSchema) {
// Report only for recognized commands
const telemetryPayload = await generateTelemetryPayload({
const telemetryPayload = generateTelemetryPayload({
command,
options: cliOptions,
commandSchema,
@ -195,7 +195,7 @@ module.exports = async (exception, options = {}) => {
if (!isUserError || !exception.code || !isErrorCodeNormative(exception.code)) {
failureReason.location = resolveErrorLocation(exceptionTokens);
}
await storeTelemetryLocally({ ...telemetryPayload, failureReason, outcome: 'failure' });
storeTelemetryLocally({ ...telemetryPayload, failureReason, outcome: 'failure' });
await sendTelemetry();
}
}

View File

@ -13,7 +13,7 @@ const ServerlessError = require('../serverless-error');
const serverlessPath = path.resolve(__dirname, '../..');
module.exports = async () => {
const localServerlessPath = await resolveLocalServerlessPath();
const localServerlessPath = resolveLocalServerlessPath();
if (localServerlessPath) {
// If the version is already local, do not try to fallback for version resolution to avoid falling into the loop

View File

@ -2,18 +2,15 @@
const path = require('path');
const memoizee = require('memoizee');
const resolve = require('ncjsm/resolve');
const resolveSync = require('ncjsm/resolve/sync');
module.exports = memoizee(
async () => {
try {
return path.resolve(
path.dirname((await resolve(process.cwd(), 'serverless')).realPath),
'..'
);
} catch {
return null;
}
},
{ promise: true }
);
// This method should be kept as sync. The reason for it is the fact that
// telemetry generation and persistence needs to be run in sync manner
// and it depends on this function, either directly or indirectly.
module.exports = memoizee(() => {
try {
return path.resolve(path.dirname(resolveSync(process.cwd(), 'serverless').realPath), '..');
} catch {
return null;
}
});

View File

@ -91,7 +91,7 @@ class Config {
throw new ServerlessError(noSupportErrorMessage, 'AUTO_UPDATE_NOT_SUPPORTED');
}
} else {
if (!(await isNpmGlobalPackage())) {
if (!isNpmGlobalPackage()) {
throw new ServerlessError(noSupportErrorMessage, 'AUTO_UPDATE_NOT_SUPPORTED');
}
if (!(await isNpmPackageWritable())) {

View File

@ -90,7 +90,7 @@ module.exports = async (serverless) => {
if (serverless.isLocallyInstalled) return;
if (serverless.isStandaloneExecutable) {
if (process.platform === 'win32') return;
} else if (!(await isNpmGlobalPackage())) {
} else if (!isNpmGlobalPackage()) {
return;
}
const currentVersionData = semver.parse(currentVersion);

View File

@ -5,7 +5,10 @@ const resolveLocalServerlessPath = require('../cli/resolve-local-serverless-path
const serverlessPath = path.resolve(__dirname, '../..');
module.exports = async () => {
const localServerlessPath = await resolveLocalServerlessPath();
// This method should be kept as sync. The reason for it is the fact that
// telemetry generation and persistence needs to be run in sync manner
// and it depends on this function, either directly or indirectly.
module.exports = () => {
const localServerlessPath = resolveLocalServerlessPath();
return serverlessPath === localServerlessPath;
};

View File

@ -2,21 +2,21 @@
const memoizee = require('memoizee');
const path = require('path');
const spawn = require('child-process-ext/spawn');
const { spawnSync } = require('child_process');
const serverlessPackageRoot = path.resolve(__dirname, '../../../');
module.exports = memoizee(
async () => {
const npmPackagesRoot = await (async () => {
try {
return String((await spawn('npm', ['root', '-g'])).stdoutBuffer).trim();
} catch {
return null;
}
})();
if (!npmPackagesRoot) return false;
return path.resolve(npmPackagesRoot, 'serverless') === serverlessPackageRoot;
},
{ type: 'promise' }
);
// This method should be kept as sync. The reason for it is the fact that
// telemetry generation and persistence needs to be run in sync manner
// and it depends on this function, either directly or indirectly.
module.exports = memoizee(() => {
const npmPackagesRoot = (() => {
try {
return String(spawnSync('npm', ['root', '-g']).stdout).trim();
} catch {
return null;
}
})();
if (!npmPackagesRoot) return false;
return path.resolve(npmPackagesRoot, 'serverless') === serverlessPackageRoot;
});

View File

@ -2,8 +2,8 @@
const path = require('path');
const os = require('os');
const fs = require('fs').promises;
const resolve = require('ncjsm/resolve');
const fs = require('fs');
const resolveSync = require('ncjsm/resolve/sync');
const _ = require('lodash');
const isPlainObject = require('type/plain-object/is');
const isObject = require('type/object/is');
@ -19,11 +19,11 @@ const AWS = require('aws-sdk');
const configValidationModeValues = new Set(['off', 'warn', 'error']);
const checkIsTabAutocompletionInstalled = async () => {
const checkIsTabAutocompletionInstalled = () => {
try {
return (await fs.readdir(path.resolve(os.homedir(), '.config/tabtab'))).some((filename) =>
filename.startsWith('serverless.')
);
return fs
.readdirSync(path.resolve(os.homedir(), '.config/tabtab'))
.some((filename) => filename.startsWith('serverless.'));
} catch {
return false;
}
@ -124,7 +124,9 @@ const getServiceConfig = ({ configuration, options }) => {
return result;
};
module.exports = async ({
// This method is explicitly kept as synchronous. The reason for it being the fact that it needs to
// be executed in such manner due to its use in `process.on('SIGINT')` handler.
module.exports = ({
command,
options,
commandSchema,
@ -176,7 +178,7 @@ module.exports = async ({
return userConfig.get('userId');
})();
const isLocallyInstalled = await (async () => {
const isLocallyInstalled = (() => {
if (serverless) {
return serverless.isLocallyInstalled;
}
@ -184,31 +186,29 @@ module.exports = async ({
return resolveIsLocallyInstalled();
})();
const usedVersions = await (async () => {
if (!isLocallyInstalled || (await resolveIsLocallyInstalled())) {
const usedVersions = (() => {
if (!isLocallyInstalled || resolveIsLocallyInstalled()) {
return {
'serverless': require('../../../package').version,
'@serverless/dashboard-plugin': require('@serverless/dashboard-plugin/package').version,
};
}
const localServerlessPath = await resolveLocalServerlessPath();
const localServerlessPath = resolveLocalServerlessPath();
return {
'serverless': require(path.resolve(localServerlessPath, 'package.json')).version,
// Since v2.42.0 it's "@serverless/dashboard-plugin"
'@serverless/dashboard-plugin': await (async () => {
'@serverless/dashboard-plugin': (() => {
try {
return require((
await resolve(localServerlessPath, '@serverless/dashboard-plugin/package')
).realPath).version;
return require(resolveSync(localServerlessPath, '@serverless/dashboard-plugin/package')
.realPath).version;
} catch {
return undefined;
}
})(),
'@serverless/enterprise-plugin': await (async () => {
'@serverless/enterprise-plugin': (() => {
try {
return require((
await resolve(localServerlessPath, '@serverless/enterprise-plugin/package')
).realPath).version;
return require(resolveSync(localServerlessPath, '@serverless/enterprise-plugin/package')
.realPath).version;
} catch {
return undefined;
}
@ -230,19 +230,19 @@ module.exports = async ({
},
firstLocalInstallationTimestamp: userConfig.get('meta.created_at'),
frameworkLocalUserId: userConfig.get('frameworkId'),
installationType: await (async () => {
installationType: (() => {
if (isStandalone) {
if (process.platform === 'win32') return 'global:standalone:windows';
return 'global:standalone:other';
}
if (!isLocallyInstalled) {
return (await isNpmGlobal()) ? 'global:npm' : 'global:other';
return isNpmGlobal() ? 'global:npm' : 'global:other';
}
if (serverless && serverless.isInvokedByGlobalInstallation) return 'local:fallback';
return 'local:direct';
})(),
isAutoUpdateEnabled: Boolean(userConfig.get('autoUpdate.enabled')),
isTabAutocompletionInstalled: await checkIsTabAutocompletionInstalled(),
isTabAutocompletionInstalled: checkIsTabAutocompletionInstalled(),
timestamp: Date.now(),
timezone,
triggeredDeprecations: Array.from(triggeredDeprecations),

View File

@ -89,7 +89,9 @@ async function request(payload, { ids, timeout } = {}) {
return processResponseBody(response, ids, startTime);
}
async function storeLocally(payload, options = {}) {
// This method is explicitly kept as synchronous. The reason for it being the fact that it needs to
// be executed in such manner due to its use in `process.on('SIGINT')` handler.
function storeLocally(payload, options = {}) {
ensurePlainObject(payload);
if (!telemetryUrl) return null;
const isForced = options && options.isForced;
@ -97,13 +99,13 @@ async function storeLocally(payload, options = {}) {
if (!cacheDirPath) return null;
const id = uuid();
return (async function self() {
return (function self() {
try {
return await fse.writeJson(join(cacheDirPath, id), { payload, timestamp: Date.now() });
return fse.writeJsonSync(join(cacheDirPath, id), { payload, timestamp: Date.now() });
} catch (error) {
if (error.code === 'ENOENT') {
try {
await fse.ensureDir(cacheDirPath);
fse.ensureDirSync(cacheDirPath);
return self();
} catch (ensureDirError) {
logError('Cache dir creation error:', ensureDirError);

View File

@ -10,37 +10,35 @@ const truthyStr = (val) => val && !['0', 'false', 'f', 'n', 'no'].includes(val.t
const { CI, ADBLOCK, SILENT } = process.env;
const isNpmGlobalPackage = require('../lib/utils/npmPackage/isGlobal');
(async () => {
if (!truthyStr(CI) && !truthyStr(ADBLOCK) && !truthyStr(SILENT)) {
const messageTokens = ['Serverless Framework successfully installed!'];
if (!truthyStr(CI) && !truthyStr(ADBLOCK) && !truthyStr(SILENT)) {
const messageTokens = ['Serverless Framework successfully installed!'];
if (isStandaloneExecutable && !isWindows) {
messageTokens.push(
'To start your first project, please open another terminal and run “serverless”.'
);
} else {
messageTokens.push('To start your first project run “serverless”.');
}
if ((isStandaloneExecutable && !isWindows) || (await isNpmGlobalPackage())) {
messageTokens.push('Turn on automatic updates by running “serverless config --autoupdate”.');
}
if (isStandaloneExecutable && !isWindows) {
messageTokens.push('Uninstall at any time by running “serverless uninstall”.');
}
const message = messageTokens.join('\n\n');
process.stdout.write(
`${
isStandaloneExecutable && isWindows
? message
: boxen(chalk.yellow(message), {
padding: 1,
margin: 1,
borderColor: 'yellow',
})
}\n`
if (isStandaloneExecutable && !isWindows) {
messageTokens.push(
'To start your first project, please open another terminal and run “serverless”.'
);
} else {
messageTokens.push('To start your first project run “serverless”.');
}
})();
if ((isStandaloneExecutable && !isWindows) || isNpmGlobalPackage()) {
messageTokens.push('Turn on automatic updates by running “serverless config --autoupdate”.');
}
if (isStandaloneExecutable && !isWindows) {
messageTokens.push('Uninstall at any time by running “serverless uninstall”.');
}
const message = messageTokens.join('\n\n');
process.stdout.write(
`${
isStandaloneExecutable && isWindows
? message
: boxen(chalk.yellow(message), {
padding: 1,
margin: 1,
borderColor: 'yellow',
})
}\n`
);
}

View File

@ -419,8 +419,8 @@ const processSpanPromise = (async () => {
hasTelemetryBeenReported = true;
if (!isTelemetryDisabled) {
await storeTelemetryLocally(
await generateTelemetryPayload({
storeTelemetryLocally(
generateTelemetryPayload({
command,
options,
commandSchema,
@ -702,15 +702,15 @@ const processSpanPromise = (async () => {
hasTelemetryBeenReported = true;
if (!isTelemetryDisabled && !isHelpRequest && serverless.isTelemetryReportedExternally) {
await storeTelemetryLocally({
...(await generateTelemetryPayload({
storeTelemetryLocally({
...generateTelemetryPayload({
command,
options,
commandSchema,
serviceDir,
configuration,
serverless,
})),
}),
outcome: 'success',
});
let backendNotificationRequest;

View File

@ -102,7 +102,7 @@ describe('test/unit/lib/cli/handle-error.test.js', () => {
});
describe('with mocked telemetry', () => {
const generateTelemetryPayloadStub = sinon.stub().resolves({});
const generateTelemetryPayloadStub = sinon.stub().returns({});
const storeTelemetryLocallyStub = sinon.stub();
const sendTelemetryStub = sinon.stub();
const resolveInputStub = sinon.stub().returns({ commandSchema: {} });

View File

@ -8,9 +8,10 @@ const fse = require('fs-extra');
const resolveLocalServerless = require('../../../../lib/cli/resolve-local-serverless-path');
describe('test/unit/lib/cli/resolve-local-serverless.test.js', () => {
it('should resolve with `null` when no local installation is found', async () => {
expect(await resolveLocalServerless()).to.equal(null);
it('should resolve with `null` when no local installation is found', () => {
expect(resolveLocalServerless()).to.equal(null);
});
it('should resolve with `null` when no local installation is found', async () => {
resolveLocalServerless.delete();
const tmpServerlessPath = path.resolve(
@ -25,6 +26,6 @@ describe('test/unit/lib/cli/resolve-local-serverless.test.js', () => {
JSON.stringify({ main: 'lib/Serverless.js' })
),
]);
expect(await fsp.realpath(await resolveLocalServerless())).to.equal(tmpServerlessPath);
expect(await fsp.realpath(resolveLocalServerless())).to.equal(tmpServerlessPath);
});
});

View File

@ -1151,7 +1151,7 @@ aws_secret_access_key = CUSTOMSECRET
});
const modulesCacheStub = {
'child-process-ext/spawn': spawnExtStub,
'./lib/utils/telemetry/generatePayload.js': async () => ({}),
'./lib/utils/telemetry/generatePayload.js': () => ({}),
};
beforeEach(() => {

View File

@ -93,7 +93,7 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
cwd: serviceDir,
command: 'print',
});
const payload = await generatePayload({
const payload = generatePayload({
command: 'print',
options: {},
commandSchema: commandsSchema.get('print'),
@ -155,7 +155,7 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
fixture: 'customProvider',
command: 'print',
});
const payload = await generatePayload({
const payload = generatePayload({
command: 'print',
options: {},
commandSchema: commandsSchema.get('print'),
@ -217,7 +217,7 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
command: 'print',
modulesCacheStub: {},
});
const payload = await overrideCwd(serviceDir, async () =>
const payload = overrideCwd(serviceDir, () =>
generatePayload({
command: 'print',
options: {},
@ -274,7 +274,7 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
});
it('Should resolve service-agnostic payload', async () => {
const payload = await generatePayload({
const payload = generatePayload({
command: 'config',
options: {},
commandSchema: commandsSchema.get('config'),
@ -308,8 +308,8 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
});
});
it('Should resolve service-agnostic payload for command with `serviceDependencyMode: "optional"`', async () => {
const payload = await generatePayload({
it('Should resolve service-agnostic payload for command with `serviceDependencyMode: "optional"`', () => {
const payload = generatePayload({
command: '',
options: {},
commandSchema: commandsSchema.get(''),
@ -358,8 +358,8 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
});
});
it('Should correctly resolve payload with missing service configuration', async () => {
const payload = await generatePayload({
it('Should correctly resolve payload with missing service configuration', () => {
const payload = generatePayload({
command: 'plugin list',
options: {},
commandSchema: commandsSchema.get('plugin list'),
@ -403,7 +403,7 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
})
);
const payload = await generatePayload({
const payload = generatePayload({
command: 'config',
options: {},
commandSchema: commandsSchema.get('config'),
@ -426,8 +426,8 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
let payload;
await overrideEnv({ variables: { SERVERLESS_ACCESS_KEY: 'some-key' } }, async () => {
payload = await generatePayload({
overrideEnv({ variables: { SERVERLESS_ACCESS_KEY: 'some-key' } }, () => {
payload = generatePayload({
command: 'config',
options: {},
commandSchema: commandsSchema.get('config'),
@ -439,11 +439,11 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
expect(payload.frameworkLocalUserId).to.equal('123');
});
it('Should correctly detect Serverless CI/CD', async () => {
it('Should correctly detect Serverless CI/CD', () => {
let payload;
await overrideEnv({ variables: { SERVERLESS_CI_CD: 'true' } }, async () => {
payload = await generatePayload({
overrideEnv({ variables: { SERVERLESS_CI_CD: 'true' } }, () => {
payload = generatePayload({
command: 'config',
options: {},
commandSchema: commandsSchema.get('config'),
@ -454,11 +454,11 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
expect(payload.ciName).to.equal('Serverless CI/CD');
});
it('Should correctly detect Seed CI/CD', async () => {
it('Should correctly detect Seed CI/CD', () => {
let payload;
await overrideEnv({ variables: { SEED_APP_NAME: 'some-app' } }, async () => {
payload = await generatePayload({
overrideEnv({ variables: { SEED_APP_NAME: 'some-app' } }, () => {
payload = generatePayload({
command: 'config',
options: {},
commandSchema: commandsSchema.get('config'),
@ -469,8 +469,8 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
expect(payload.ciName).to.equal('Seed');
});
it('Should correctly resolve `commandOptionNames` property', async () => {
const payload = await generatePayload({
it('Should correctly resolve `commandOptionNames` property', () => {
const payload = generatePayload({
command: 'print',
options: {
region: 'eu-west-1',
@ -487,8 +487,8 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
);
});
it('Should correctly resolve `constructs` property', async () => {
const payload = await generatePayload({
it('Should correctly resolve `constructs` property', () => {
const payload = generatePayload({
command: 'print',
commandSchema: commandsSchema.get('print'),
options: {},
@ -523,8 +523,8 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
]);
});
it('Should correctly resolve `configValidationMode` property', async () => {
const payload = await generatePayload({
it('Should correctly resolve `configValidationMode` property', () => {
const payload = generatePayload({
command: 'print',
commandSchema: commandsSchema.get('print'),
options: {},
@ -538,12 +538,12 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
expect(payload.config.configValidationMode).to.equal('off');
});
it('Should correctly resolve `hasLocalCredentials` property for AWS provider', async () => {
it('Should correctly resolve `hasLocalCredentials` property for AWS provider', () => {
let payload;
await overrideEnv(
overrideEnv(
{ variables: { AWS_ACCESS_KEY_ID: 'someaccesskey', AWS_SECRET_ACCESS_KEY: 'secretkey' } },
async () => {
payload = await generatePayload({
() => {
payload = generatePayload({
command: 'print',
options: {},
commandSchema: commandsSchema.get('print'),
@ -556,12 +556,12 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
expect(payload.hasLocalCredentials).to.equal(true);
});
it('Should correctly resolve `hasLocalCredentials` property for non-AWS provider', async () => {
it('Should correctly resolve `hasLocalCredentials` property for non-AWS provider', () => {
let payload;
await overrideEnv(
overrideEnv(
{ variables: { AWS_ACCESS_KEY_ID: 'someaccesskey', AWS_SECRET_ACCESS_KEY: 'secretkey' } },
async () => {
payload = await generatePayload({
() => {
payload = generatePayload({
command: 'print',
options: {},
commandSchema: commandsSchema.get('print'),
@ -574,8 +574,8 @@ describe('test/unit/lib/utils/telemetry/generatePayload.test.js', () => {
expect(payload.hasLocalCredentials).to.equal(false);
});
it('Should correctly resolve `commandUsage` property', async () => {
const payload = await generatePayload({
it('Should correctly resolve `commandUsage` property', () => {
const payload = generatePayload({
command: 'print',
options: {},
commandSchema: commandsSchema.get('print'),

View File

@ -62,7 +62,7 @@ describe('test/unit/lib/utils/telemetry/index.test.js', () => {
it('`storeLocally` should persist an event in cacheDir', async () => {
const payload = { test: 'payloadvalue' };
await storeLocally(payload);
storeLocally(payload);
const dirFilenames = await fse.readdir(cacheDirPath);
expect(dirFilenames.length).to.equal(1);
const persistedEvent = await fse.readJson(path.join(cacheDirPath, dirFilenames[0]));