Skip to content
This repository was archived by the owner on Apr 3, 2024. It is now read-only.

Commit 73b171e

Browse files
DominicKramerDominicKramer
authored
chore: remove the GCLOUD_USE_INSPECTOR env var (#429)
Now the inspector protocol is necessarily used for Node >= 10 and the legacy protocol is used for Node < 10.
1 parent 54d3d10 commit 73b171e

File tree

7 files changed

+27
-83
lines changed

7 files changed

+27
-83
lines changed

‎README.md

-13
Original file line numberDiff line numberDiff line change
@@ -176,19 +176,6 @@ Further, you do not need to deploy the original source files to the deployment e
176176

177177
In the Debug UI, you only need to provide the original source code -- you don't need the transpiled output files or the sourcemaps. When you set a snapshot in an original source file in the Debug UI, the corresponding file and line in the transpiled code is automatically determined based on the sourcemap files provided with the transpiled code at runtime. See the [Using the Debugger](#using-the-debugger) section for more information about using the Debug UI. In addition, the exact directory layout of the original source is somewhat flexible, just as it is with the use of non-transpiled code as described in the [Using the Debugger](#using-the-debugger) section.
178178

179-
## Experimental Features
180-
181-
The Stackdriver Debugger includes experimental support for the new [V8 Inspector Protocol](https://chromedevtools.github.io/debugger-protocol-viewer/v8/) and will use it if and only if the `GCLOUD_USE_INSPECTOR` environment variable is set and the running version of Node supports the inspector protocol (Node 8+).
182-
183-
If the `GCLOUD_USE_INSPECTOR` environment variable is set, but the running version of Node does not support the inspector protocol, the agent will fall back to the legacy debugger protocol and a warning message will be logged.
184-
185-
The `GCLOUD_USE_INSPECTOR` can be set programmatically, but must be set before calling the `start` method as illustrated below:
186-
187-
```js
188-
process.env['GCLOUD_USE_INSPECTOR'] = true;
189-
require('@google-cloud/debug-agent').start({ ... });
190-
```
191-
192179
## Using Stackdriver Debugger on Google Cloud Functions
193180

194181
The Stackdriver Debugger also introduces a new `isReady` method that returns a `Promise` that is resolved in either of the three scenarios.

‎appveyor.yml

-3
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,6 @@ install:
2020
test_script:
2121
# run tests
2222
- npm run compile
23-
- SET GCLOUD_USE_INSPECTOR=
24-
- node_modules/.bin/mocha build/test --timeout 4000 --R
25-
- SET GCLOUD_USE_INSPECTOR=1
2623
- node_modules/.bin/mocha build/test --timeout 4000 --R
2724

2825
# Don't actually build using MSBuild

‎bin/run-test.sh

+1-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ function run {
3030
}
3131

3232
# Run test/coverage
33-
GCLOUD_USE_INSPECTOR= run build/test
34-
GCLOUD_USE_INSPECTOR=true run build/test
33+
run build/test
3534

3635
# Conditionally publish coverage
3736
if [ "$cover" ]; then

‎src/agent/util/utils.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ export const messages = {
2525
CAPTURE_BREAKPOINT_DATA: 'Error trying to capture snapshot data: ',
2626
INVALID_LINE_NUMBER: 'Invalid snapshot position: ',
2727
COULD_NOT_FIND_OUTPUT_FILE:
28-
'Could not determine the output file associated with the transpiled input file',
29-
INSPECTOR_NOT_AVAILABLE:
30-
'The V8 Inspector protocol is only available in Node 8+'
28+
'Could not determine the output file associated with the transpiled input file'
3129
};
3230

3331
export interface Listener {

‎src/agent/v8/debugapi.ts

+2-7
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,9 @@ interface DebugApiConstructor {
4242

4343
let debugApiConstructor: DebugApiConstructor;
4444

45-
export function willUseInspector(nodeVersion?: string, useInspector?: boolean) {
45+
export function willUseInspector(nodeVersion?: string) {
4646
const version = nodeVersion != null ? nodeVersion : process.version;
47-
const node10Above = utils.satisfies(version, '>=10');
48-
const node8Above = utils.satisfies(version, '>=8');
49-
const resolvedUseInspector =
50-
useInspector != null ? useInspector : !!process.env.GCLOUD_USE_INSPECTOR;
51-
52-
return node10Above || (node8Above && resolvedUseInspector);
47+
return utils.satisfies(version, '>=10');
5348
}
5449

5550
if (willUseInspector()) {

‎src/agent/v8/legacy-debugapi.ts

-3
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,6 @@ export class V8DebugApi implements debugapi.DebugApi {
9090
this.logger.warn('Internal V8 error on breakpoint event: ' + e);
9191
}
9292
};
93-
if (process.env.GCLOUD_USE_INSPECTOR) {
94-
this.logger.warn(utils.messages.INSPECTOR_NOT_AVAILABLE);
95-
}
9693
if (this.usePermanentListener) {
9794
this.logger.info('activating v8 breakpoint listener (permanent)');
9895

‎test/test-v8debugapi.ts

+23-52
Original file line numberDiff line numberDiff line change
@@ -113,61 +113,39 @@ function validateBreakpoint(breakpoint: stackdriver.Breakpoint): void {
113113
breakpoint.stackFrames.forEach(validateStackFrame);
114114
}
115115
}
116+
116117
describe(
117118
'propertly determines if the inspector protocol should be used', () => {
118-
const suffixes = ['', '.11', '.11.1'];
119-
it('handles nightly builds correctly', () => {
120-
for (const suffix of suffixes) {
121-
// nightly builds should be handled correctly
122-
assert.strictEqual(
123-
debugapi.willUseInspector(
124-
'v10.0.0-nightly201804132a6ab9b37b', true),
125-
true);
126-
assert.strictEqual(
127-
debugapi.willUseInspector(
128-
'v10.0.0-nightly201804132a6ab9b37b', false),
129-
true);
130-
}
131-
});
119+
let suffixes = ['', '.11', '.11.1'];
120+
// also handle suffixes associated with nightly builds
121+
suffixes = suffixes.concat(
122+
suffixes.map(suffix => suffix + '-nightly201804132a6ab9b37b'));
132123

133124
it('handles Node >=10 correctly', () => {
134-
for (const suffix of suffixes) {
135-
// on Node >= 10, inspector should always be used
136-
assert.strictEqual(
137-
debugapi.willUseInspector(`v10${suffix}`, true), true);
138-
assert.strictEqual(
139-
debugapi.willUseInspector(`v10${suffix}`, false), true);
140-
assert.strictEqual(
141-
debugapi.willUseInspector(`v11${suffix}`, true), true);
142-
assert.strictEqual(
143-
debugapi.willUseInspector(`v11${suffix}`, false), true);
125+
// on Node >= 10, inspector should always be used
126+
for (let version = 10; version <= 11; version++) {
127+
for (const suffix of suffixes) {
128+
const fullVersion = `v${version}${suffix}`;
129+
assert.strictEqual(
130+
debugapi.willUseInspector(fullVersion), true,
131+
`Should use inspector in Node.js version ${fullVersion}`);
132+
}
144133
}
145134
});
146135

147-
it('handles Node 8 correctly', () => {
148-
for (const suffix of suffixes) {
149-
// on Node 8, inspector should only be used if explicitly specified
150-
assert.strictEqual(
151-
debugapi.willUseInspector(`v8${suffix}`, true), true);
152-
assert.strictEqual(
153-
debugapi.willUseInspector(`v8${suffix}`, false), false);
154-
}
155-
});
156-
157-
it('handles Node <8 correctly', () => {
158-
for (const suffix of suffixes) {
159-
// on Node < 8, inspector should never be used
160-
assert.strictEqual(
161-
debugapi.willUseInspector(`v6${suffix}`, true), false);
162-
assert.strictEqual(
163-
debugapi.willUseInspector(`v6${suffix}`, false), false);
164-
assert.strictEqual(
165-
debugapi.willUseInspector(`v4${suffix}`, true), false);
166-
assert.strictEqual(
167-
debugapi.willUseInspector(`v4${suffix}`, false), false);
136+
it('handles Node <10 correctly', () => {
137+
// on Node < 10, inspector should never be used
138+
for (let version = 4; version <= 9; version++) {
139+
for (const suffix of suffixes) {
140+
const fullVersion = `v${version}${suffix}`;
141+
assert.strictEqual(
142+
debugapi.willUseInspector(fullVersion), false,
143+
`Should not use inspector in Node.js version ${fullVersion}`);
144+
}
168145
}
169146
});
170147
});
148+
171149
describe('debugapi selection', () => {
172150
const config: ResolvedDebugAgentConfig = extend(
173151
{}, defaultConfig, {workingDirectory: __dirname, forceNewAgent_: true});
@@ -199,12 +177,6 @@ describe('debugapi selection', () => {
199177
const v8debugapi = require('../src/agent/v8/legacy-debugapi');
200178
assert.ok(api instanceof v8debugapi.V8DebugApi);
201179
}
202-
if (process.env.GCLOUD_USE_INSPECTOR &&
203-
utils.satisfies(process.version, '<8')) {
204-
assert(logText.includes(utils.messages.INSPECTOR_NOT_AVAILABLE));
205-
} else {
206-
assert(!logText.includes(utils.messages.INSPECTOR_NOT_AVAILABLE));
207-
}
208180
done();
209181
});
210182
});
@@ -237,7 +209,6 @@ describeFn('debugapi selection on Node >=10', () => {
237209
api = debugapi.create(logger, config, jsStats, mapper!);
238210
const inspectorapi = require('../src/agent/v8/inspector-debugapi');
239211
assert.ok(api instanceof inspectorapi.InspectorDebugApi);
240-
assert(!logText.includes(utils.messages.INSPECTOR_NOT_AVAILABLE));
241212
done();
242213
});
243214
});

0 commit comments

Comments
 (0)