Skip to content

Commit

Permalink
chore: prevent profiler from being used with versions of node 10 with…
Browse files Browse the repository at this point in the history
… GC pauses (#242)

* chore: prevent profiler from being used with versions of node 10 with GC bug

* Update and link to docs

* add tests for node semver limit
  • Loading branch information
nolanmar511 committed Jul 17, 2018
1 parent 4ecef31 commit a4b7e6e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 9 deletions.
17 changes: 15 additions & 2 deletions README.md
Expand Up @@ -14,8 +14,21 @@ deprecation policy.*

## Prerequisites

1. Your application will need to be using Node.js version 6.12.3 or greater,
or Node.js 8.9.4 or greater.
1. Your application will need to be using Node.js version 6.12.3 or greater,
Node.js 8.9.4 or greater, or Node.js 10.4.1 or greater. The profiler will not
be enabled when using earlier versions of Node 6, 8, and 10 because the
profiler is not stable with those versions of Node.js.
* Versions of Node.js 6 prior to 6.12.3 are impacted by
[this](https://bugs.chromium.org/p/v8/issues/detail?id=4959) issue, which can
cause segmentation faults when heap profiling is enabled.
* Versions of Node.js before Node.js 8 prior to 8.9.4 are impacted by
[this](https://bugs.chromium.org/p/v8/issues/detail?id=6623) issue, which
causes a memory leak when time profiling is enabled.
* Versions of Node.js 10 prior to 10.4.1 are impacted by
[this](https://bugs.chromium.org/p/chromium/issues/detail?id=847863) issue,
which can cause garbage collection to take several minutes when heap
profiling is enabled.

1. You will need a project in the [Google Developers Console][cloud-console].
Your application can run anywhere, but the profiler data is associated with a
particular project.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -82,6 +82,6 @@
]
},
"engines": {
"node": "6.12.3 - 7.0.0 || >=8.9.4"
"node": ">=6.12.3 <8.0.0 || >=8.9.4 <10.0.0 || >=10.4.1"
}
}
24 changes: 19 additions & 5 deletions ts/src/index.ts
Expand Up @@ -23,6 +23,7 @@ import * as path from 'path';
import {normalize} from 'path';
import * as pify from 'pify';
import * as semver from 'semver';
import {SemVer} from 'semver';

import {Config, defaultConfig, ProfilerConfig} from './config';
import {Profiler} from './profiler';
Expand Down Expand Up @@ -121,19 +122,32 @@ async function initConfigMetadata(config: ProfilerConfig):
return config;
}


/**
* Returns true if the version passed in satifised version requirements
* specified in the profiler's package.json.
*
* Exported for testing.
*/
export function nodeVersionOkay(version: string|SemVer): boolean {
// Coerce version if possible, to remove any pre-release, alpha, beta, etc
// tags.
version = semver.coerce(version) || version;
return semver.satisfies(version, pjson.engines.node);
}

/**
* Initializes the config, and starts heap profiler if the heap profiler is
* needed. Returns a profiler if creation is successful. Otherwise, returns
* rejected promise.
*/
export async function createProfiler(config: Config): Promise<Profiler> {
// Coerce version if possible, to remove any pre-release, alpha, beta, etc
// tags.
const version = semver.coerce(process.version) || process.version;
if (!semver.satisfies(version, pjson.engines.node)) {
if (!nodeVersionOkay(process.version)) {
throw new Error(
`Could not start profiler: node version ${process.version}` +
` does not satisfies "${pjson.engines.node}"`);
` does not satisfies "${pjson.engines.node}"` +
'\nSee https://github.com/GoogleCloudPlatform/cloud-profiler-nodejs#prerequisites' +
' for details.');
}

let profilerConfig: ProfilerConfig = initConfigLocal(config);
Expand Down
41 changes: 40 additions & 1 deletion ts/test/test-init-config.ts
Expand Up @@ -21,12 +21,51 @@ import * as extend from 'extend';
import * as gcpMetadata from 'gcp-metadata';
import * as sinon from 'sinon';

import {createProfiler} from '../src/index';
import {createProfiler, nodeVersionOkay} from '../src/index';
import {Profiler} from '../src/profiler';
import * as heapProfiler from '../src/profilers/heap-profiler';

const v8HeapProfiler = require('bindings')('sampling_heap_profiler');

describe('nodeVersionOkay', () => {
it('should accept alpha versions', () => {
assert.equal(true, nodeVersionOkay('v11.0.0-alpha.1'));
});
it('should accept beta versions', () => {
assert.equal(true, nodeVersionOkay('v8.9.10-beta.2'));
});
it('should accept nightly versions', () => {
assert.equal(true, nodeVersionOkay('v11.0.0-nightly2018000000'));
});
it('should accept pre-release versions', () => {
assert.equal(true, nodeVersionOkay('v11.0.0-pre'));
});
it('should accept v6.12.3', () => {
assert.equal(true, nodeVersionOkay('v6.12.3'));
});
it('should not accept v6.12.2', () => {
assert.equal(false, nodeVersionOkay('v6.12.2'));
});
it('should accept v8.9.4', () => {
assert.equal(true, nodeVersionOkay('v8.9.4'));
});
it('should not accept v8.9.3', () => {
assert.equal(false, nodeVersionOkay('v8.9.3'));
});
it('should accept v10.4.1', () => {
assert.equal(true, nodeVersionOkay('v10.4.1'));
});
it('should not accept v10.4.0', () => {
assert.equal(false, nodeVersionOkay('v10.4.0'));
});
it('should accept node 7', () => {
assert.equal(true, nodeVersionOkay('v7.7.7'));
});
it('should accept node 9', () => {
assert.equal(true, nodeVersionOkay('v9.9.9'));
});
});

describe('createProfiler', () => {
let savedEnv: NodeJS.ProcessEnv;
let metadataStub: sinon.SinonStub|undefined;
Expand Down

0 comments on commit a4b7e6e

Please sign in to comment.