Skip to content

Commit 6e68939

Browse files
BFergersonMrMineO5
BFergerson
and
MrMineO5
authored
error handling (#5)
* Implement log functionality * Add error handling * Initialize ContextReceiver.logReport after loading config Co-authored-by: UltraDev <petzmagnus@gmail.com>
1 parent 4e524b8 commit 6e68939

File tree

10 files changed

+240
-89
lines changed

10 files changed

+240
-89
lines changed

‎gradle.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
kotlin.code.style=official
22

33
probeGroup=plus.sourceplus.probe
4-
projectVersion=0.6.3-SNAPSHOT
4+
projectVersion=0.6.8-SNAPSHOT

‎src/SourcePlusPlus.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ namespace SourcePlusPlus {
130130
async function sendConnected(eventBus: EventBus): Promise<void> {
131131
let probeMetadata = {
132132
language: 'nodejs',
133-
probe_version: '1.0.0', // TODO
133+
probe_version: '1.0.3', // TODO
134134
nodejs_version: process.version,
135135
service: config.serviceName,
136136
service_instance: config.serviceInstance,

‎src/control/ContextReceiver.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,14 @@ import VariableUtil from "../util/VariableUtil";
1010
import ProbeMemory from "../ProbeMemory";
1111

1212
namespace ContextReceiver {
13-
let logReport = new LogReportServiceClient(
14-
config.collectorAddress,
15-
config.secure ? grpc.credentials.createSsl() : grpc.credentials.createInsecure()
16-
);
13+
let logReport;
14+
15+
export function initialize() {
16+
logReport = new LogReportServiceClient(
17+
config.collectorAddress,
18+
config.secure ? grpc.credentials.createSsl() : grpc.credentials.createInsecure()
19+
);
20+
}
1721

1822
function tryFindVariable(varName, variables) {
1923
for (let scope in variables) {
@@ -89,17 +93,17 @@ namespace ContextReceiver {
8993
activeSpan.stop();
9094
}
9195

92-
export function applyLog(liveLogId: string, logFormat: string, logArguments: string[], variables) {
96+
export function applyLog(liveLogId: string, logFormat: string, logArguments: any) {
9397
let logTags = new LogTags();
9498
logTags.addData(new KeyStringValuePair().setKey('log_id').setValue(liveLogId));
9599
logTags.addData(new KeyStringValuePair().setKey('level').setValue('Live'));
96100
logTags.addData(new KeyStringValuePair().setKey('thread').setValue('n/a'));
97101

98-
for (let varName of logArguments) {
99-
let variable = tryFindVariable(varName, variables);
100-
let value = variable ? variable.value : "null"; // TODO: Properly toString the variable (or encode it)
101-
if (variable) {
102-
logTags.addData(new KeyStringValuePair().setKey(`argument.${varName}`).setValue(value));
102+
if (logArguments) {
103+
for (const varName in logArguments) {
104+
logTags.addData(new KeyStringValuePair()
105+
.setKey(`argument.${varName}`)
106+
.setValue(logArguments[varName]));
103107
}
104108
}
105109

‎src/control/LiveInstrumentRemote.ts‎

Lines changed: 84 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ export default class LiveInstrumentRemote {
4343
}
4444

4545
this.sourceMapper = new SourceMapper(this.scriptLoaded.bind(this));
46+
47+
ContextReceiver.initialize();
4648
}
4749

4850
async start(): Promise<void> {
@@ -93,84 +95,87 @@ export default class LiveInstrumentRemote {
9395
}));
9496
}
9597

96-
let instrumentIds = this.breakpointIdToInstrumentIds.get(message.params.hitBreakpoints[0]); // TODO: Handle multiple hit breakpoints
97-
if (!instrumentIds) {
98-
this.removeBreakpoint(message.params.hitBreakpoints[0]);
99-
return;
100-
}
98+
message.params.hitBreakpoints.forEach(breakpointId => { // There should only be a single breakpoint, but handle multiple just in case
99+
let instrumentIds = this.breakpointIdToInstrumentIds.get(breakpointId);
100+
if (!instrumentIds) {
101+
this.removeBreakpoint(message.params.hitBreakpoints[0]);
102+
return;
103+
}
101104

102-
let instruments = instrumentIds.map(id => this.instruments.get(id));
103-
let conditionsSatisfied = Promise.all(instruments.map(instrument => {
104-
if (instrument.condition === undefined || instrument.condition === null)
105-
return true;
106-
107-
return new Promise<boolean>((resolve, reject) => {
108-
this.session.post("Debugger.evaluateOnCallFrame", {
109-
callFrameId: frame.callFrameId,
110-
expression: instrument.condition,
111-
silent: false,
112-
throwOnSideEffect: true
113-
}, (err, res) => {
114-
if (err) {
115-
console.log(`Error evaluating condition (${instrument.condition}): ${err}`);
116-
resolve(false);
117-
} else {
118-
if (res.result.type === 'object' && res.result.subtype === 'error') {
119-
if (res.result.className === 'EvalError') {
120-
console.log(`Could not evaluate condition (${instrument.condition}) due to possible side effects`);
105+
let instruments = instrumentIds.map(id => this.instruments.get(id));
106+
let dataGathered = Promise.all(instruments.map(instrument => {
107+
return new Promise<any>((resolve, reject) => {
108+
this.session.post("Debugger.evaluateOnCallFrame", {
109+
callFrameId: frame.callFrameId,
110+
expression: instrument.createExpression(),
111+
silent: false, // In case of an exception, don't affect the program flow
112+
throwOnSideEffect: true, // Disallow side effects
113+
returnByValue: true // Return the entire JSON object rather than just the remote id
114+
}, (err, res) => {
115+
if (err) {
116+
this.handleConditionalFailed(instrument,
117+
`Error evaluating condition (${instrument.condition}): ${err}`);
118+
resolve({success: false});
119+
} else {
120+
if (res.result.type === 'object' && res.result.subtype === 'error') {
121+
if (res.result.className === 'EvalError') {
122+
this.handleConditionalFailed(instrument,
123+
`Could not evaluate condition (${instrument.condition}) due to possible side effects`);
124+
} else {
125+
this.handleConditionalFailed(instrument,
126+
`Error evaluating condition (${instrument.condition}): ${res.result.description}`);
127+
}
128+
resolve({success: false});
129+
} else if (res.result.type !== 'object') {
130+
this.handleConditionalFailed(instrument,
131+
`Invalid condition for instrument id: ${instrument.id}: ${instrument.condition} ==> ${res.result}`);
132+
resolve({success: false});
121133
} else {
122-
console.log(`Error evaluating condition (${instrument.condition}): ${res.result.description}`);
134+
resolve(res.result.value);
123135
}
124-
resolve(false);
125-
} else if (res.result.type !== 'boolean') {
126-
console.log("Invalid condition for instrument id: " + instrument.id + ": " + instrument.condition, res.result);
127-
resolve(false);
128-
} else {
129-
resolve(res.result.value);
130136
}
131-
}
137+
});
132138
});
133-
});
134-
}));
135-
136-
Promise.all(promises).then(() => conditionsSatisfied)
137-
.then(conditions => {
138-
for (let i = 0; i < instruments.length; i++) {
139-
if (conditions[i]) {
140-
let instrument = instruments[i];
141-
if (!instrument) {
142-
continue;
143-
}
139+
}));
140+
141+
Promise.all(promises).then(() => dataGathered)
142+
.then(data => {
143+
for (let i = 0; i < instruments.length; i++) {
144+
if (data[i].success) {
145+
let instrument = instruments[i];
146+
if (!instrument) {
147+
continue;
148+
}
144149

145-
if (instrument.type == LiveInstrumentType.BREAKPOINT) {
146-
ContextReceiver.applyBreakpoint(
147-
instrument.id,
148-
instrument.location.source,
149-
instrument.location.line,
150-
message.params.callFrames,
151-
variables
152-
);
153-
} else if (instrument.type == LiveInstrumentType.LOG) {
154-
let logInstrument = <LiveLog>instrument;
155-
ContextReceiver.applyLog(
156-
instrument.id,
157-
logInstrument.logFormat,
158-
logInstrument.logArguments,
159-
variables
160-
);
161-
} else if (instrument.type == LiveInstrumentType.METER) {
162-
let meterInstrument = <LiveMeter>instrument;
163-
ContextReceiver.applyMeter(
164-
instrument.id,
165-
variables
166-
);
167-
}
168-
if (instrument.isFinished()) {
169-
this.removeBreakpoint(instrument.id);
150+
if (instrument.type == LiveInstrumentType.BREAKPOINT) {
151+
ContextReceiver.applyBreakpoint(
152+
instrument.id,
153+
instrument.location.source,
154+
instrument.location.line,
155+
message.params.callFrames,
156+
variables
157+
);
158+
} else if (instrument.type == LiveInstrumentType.LOG) {
159+
let logInstrument = <LiveLog>instrument;
160+
ContextReceiver.applyLog(
161+
instrument.id,
162+
logInstrument.logFormat,
163+
data[i].logArguments
164+
);
165+
} else if (instrument.type == LiveInstrumentType.METER) {
166+
let meterInstrument = <LiveMeter>instrument;
167+
ContextReceiver.applyMeter(
168+
instrument.id,
169+
variables
170+
);
171+
}
172+
if (instrument.isFinished()) {
173+
this.removeBreakpoint(instrument.id);
174+
}
170175
}
171176
}
172-
}
173-
});
177+
});
178+
});
174179
});
175180
}
176181

@@ -332,6 +337,15 @@ export default class LiveInstrumentRemote {
332337
}
333338
}
334339

340+
handleConditionalFailed(instrument: LiveInstrument, error: string) {
341+
this.removeInstrument(instrument.id);
342+
this.eventBus.publish("spp.processor.status.live-instrument-removed", {
343+
occurredAt: Date.now(),
344+
instrument: JSON.stringify(instrument.toJson()),
345+
cause: `EventBusException:LiveInstrumentException[CONDITIONAL_FAILED]: ${error}`
346+
});
347+
}
348+
335349
// TODO: Call this regularly to clean up old instruments
336350
// TODO: Ensure the cache doesn't get too large
337351
private cleanCache() {

‎src/model/LiveInstrument.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,11 @@ export default class LiveInstrument {
4040
meta: this.meta
4141
};
4242
}
43+
44+
createExpression() {
45+
if (this.condition == null) {
46+
return `(() => { return {success: true} })()`;
47+
}
48+
return `(() => { return {success: ${this.condition}} })()`;
49+
}
4350
}

‎src/model/error/LiveInstrumentException.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
export class LiveInstrumentException {
2+
type: ErrorType;
3+
message: string;
4+
5+
toEventBusString(): string {
6+
return `EventBusException:LiveInstrumentException[${ErrorType[this.type]}]: ${this.message}`;
7+
}
8+
}
9+
10+
export enum ErrorType {
11+
CLASS_NOT_FOUND,
12+
CONDITIONAL_FAILED
13+
}

‎src/model/instruments/LiveLog.ts‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,31 @@
11
import LiveInstrument from "../LiveInstrument";
2-
import LiveSourceLocation from "../LiveSourceLocation";
3-
import InstrumentThrottle from "../throttle/InstrumentThrottle";
42
import LiveInstrumentType from "../LiveInstrumentType";
5-
import HitThrottle from "../throttle/HitThrottle";
63

74
export default class LiveLog extends LiveInstrument {
85
type = LiveInstrumentType.LOG;
96
logFormat: string
107
logArguments: string[]
8+
9+
createExpression(): string {
10+
let logArgumentsExpression = this.logArguments
11+
.map(arg => `data['${arg}'] = ${arg}.toString()`)
12+
.join(';');
13+
if (this.condition == null) {
14+
return `(() => {
15+
let data = {success: true};
16+
(data => {${logArgumentsExpression}})(data);
17+
return data;
18+
})()`;
19+
} else {
20+
return `(() => {
21+
if (${this.condition}) {
22+
let data = {success: true};
23+
(data => {${logArgumentsExpression}})(data);
24+
returndata;
25+
} else {
26+
return {success: false};
27+
}
28+
})()`;
29+
}
30+
}
1131
}

‎test/LiveLogTest.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
const assert = require('assert');
2+
const TestUtils = require("./TestUtils.js");
3+
4+
module.exports = function () {
5+
function simplePrimitives() {
6+
let i = 1
7+
let c = 'h'
8+
let s = "hi"
9+
let f = 1.0
10+
let bool = true
11+
TestUtils.addLineLabel("done", () => TestUtils.getLineNumber())
12+
}
13+
14+
it('add live log', async function () {
15+
simplePrimitives() //setup labels
16+
17+
await TestUtils.addLiveLog({
18+
"source": TestUtils.getFilename()(),
19+
"line": TestUtils.getLineLabelNumber("done")
20+
}, null, 1, "test log", []).then(function (res) {
21+
assert.equal(res.status, 200);
22+
simplePrimitives(); //trigger breakpoint
23+
}).catch(function (err) {
24+
assert.fail(err)
25+
});
26+
});
27+
28+
it('verify log data', async function () {
29+
this.timeout(2000)
30+
31+
setTimeout(() => simplePrimitives(), 1000);
32+
33+
let event = await TestUtils.awaitMarkerEvent("LOG_HIT");
34+
console.log(event);
35+
// assert.equal(event.stackTrace.elements[0].method, 'simplePrimitives');
36+
// let variables = event.stackTrace.elements[0].variables;
37+
//
38+
// let iVar = TestUtils.locateVariable("i", variables);
39+
// assert.equal(iVar.liveClazz, "number");
40+
// assert.equal(iVar.value, 1);
41+
//
42+
// let cVar = TestUtils.locateVariable("c", variables);
43+
// assert.equal(cVar.liveClazz, "string");
44+
// assert.equal(cVar.value, "h");
45+
//
46+
// let sVar = TestUtils.locateVariable("s", variables);
47+
// assert.equal(sVar.liveClazz, "string");
48+
// assert.equal(sVar.value, "hi");
49+
//
50+
// let fVar = TestUtils.locateVariable("f", variables);
51+
// assert.equal(fVar.liveClazz, "number");
52+
// assert.equal(fVar.value, 1.0);
53+
//
54+
// let boolVar = TestUtils.locateVariable("bool", variables);
55+
// assert.equal(boolVar.liveClazz, "boolean");
56+
// assert.equal(boolVar.value, true);
57+
});
58+
};

0 commit comments

Comments
 (0)