Skip to content

Commit 18d9532

Browse files
felixfbeckerfelixfbecker
committed
Improve error handling
 - DBGPConnection now inherits from EventEmitter and error events are handled by PHPDebugSession - XMLDOM Errors are now handled, warnings ignored - Logs are not send with stdout/stderr but the default value (console) - sendErrorResponse is extended to accept an error object - If breakpoint creation failed, the error message is returned with the breakpoint object
1 parent 1d06875 commit 18d9532

File tree

4 files changed

+76
-38
lines changed

4 files changed

+76
-38
lines changed

‎src/dbgp.ts‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,11 @@ import {DOMParser} from 'xmldom';
77
/** The encoding all XDebug messages are encoded with */
88
const ENCODING = 'iso-8859-1';
99

10-
function parseResponse(response: Buffer): XMLDocument {
11-
const xml = iconv.decode(response, ENCODING);
12-
const parser = new DOMParser();
13-
const document = parser.parseFromString(xml, 'application/xml');
14-
return document;
15-
}
16-
1710
/** The two states the connection switches between */
1811
enum ParsingState {DataLength, Response};
1912

2013
/** Wraps the NodeJS Socket and calls handleResponse() whenever a full response arrives */
21-
export abstract class DbgpConnection {
14+
export abstract class DbgpConnection extends EventEmitter {
2215

2316
private _socket: net.Socket;
2417
private _parsingState: ParsingState;
@@ -27,11 +20,13 @@ export abstract class DbgpConnection {
2720
private _dataLength: number;
2821

2922
constructor(socket: net.Socket) {
23+
super();
3024
this._socket = socket;
3125
this._parsingState = ParsingState.DataLength;
3226
this._chunksDataLength = 0;
3327
this._chunks = [];
3428
socket.on('data', (data: Buffer) => this._handleDataChunk(data));
29+
socket.on('error', (error: Error) => this.emit('error'));
3530
}
3631

3732
private _handleDataChunk(data: Buffer) {
@@ -69,7 +64,22 @@ export abstract class DbgpConnection {
6964
this._chunksDataLength += data.length;
7065
const response = Buffer.concat(this._chunks, this._chunksDataLength);
7166
// call response handler
72-
this.handleResponse(parseResponse(response));
67+
const xml = iconv.decode(response, ENCODING);
68+
const parser = new DOMParser({
69+
errorHandler: {
70+
warning: warning => {
71+
console.log(warning);
72+
},
73+
error: error => {
74+
this.emit('error', error);
75+
},
76+
fatalError: error => {
77+
this.emit('error', error);
78+
}
79+
}
80+
});
81+
const document = parser.parseFromString(xml, 'application/xml');
82+
this.handleResponse(document);
7383
// reset buffer
7484
this._chunks = [];
7585
this._chunksDataLength = 0;

‎src/phpDebug.ts‎

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ class PhpDebugSession extends vscode.DebugSession {
132132
}
133133

134134
protected attachRequest(response: VSCodeDebugProtocol.AttachResponse, args: VSCodeDebugProtocol.AttachRequestArguments) {
135-
this.sendErrorResponse(response, 0, 'Attach requests are not supported');
135+
this.sendErrorResponse(response, new Error('Attach requests are not supported'));
136136
this.shutdown();
137137
}
138138

@@ -142,8 +142,16 @@ class PhpDebugSession extends vscode.DebugSession {
142142
server.on('connection', (socket: net.Socket) => {
143143
// new XDebug connection
144144
const connection = new xdebug.Connection(socket);
145+
console.log('new connection ' + connection.id + '\n');
145146
this._connections.set(connection.id, connection);
146147
this._waitingConnections.add(connection);
148+
connection.on('error', (error: Error) => {
149+
this.sendEvent(new vscode.OutputEvent(error.message));
150+
this.sendEvent(new vscode.ThreadEvent('exited', connection.id));
151+
connection.close();
152+
this._connections.delete(connection.id);
153+
this._waitingConnections.delete(connection);
154+
});
147155
connection.waitForInitPacket()
148156
.then(() => {
149157
this.sendEvent(new vscode.ThreadEvent('started', connection.id));
@@ -153,12 +161,15 @@ class PhpDebugSession extends vscode.DebugSession {
153161
// raise default of 32
154162
.then(response => connection.sendFeatureSetCommand('max_children', '9999'))
155163
// request breakpoints from VS Code
156-
// once VS Code has set all breakpoints (eg breakpointsSet and exceptionBreakpointsSet are true) _runOrStopOnEntry will be called
157164
.then(response => this.sendEvent(new vscode.InitializedEvent()))
158165
.catch(error => {
159-
console.error('error: ', error);
166+
this.sendEvent(new vscode.OutputEvent(error.message));
160167
});
161168
});
169+
server.on('error', (error: Error) => {
170+
this.sendEvent(new vscode.OutputEvent(error.message));
171+
this.shutdown();
172+
});
162173
server.listen(args.port || 9000, () => {
163174
if (args.program) {
164175
const runtimeArgs = args.runtimeArgs || [];
@@ -176,7 +187,7 @@ class PhpDebugSession extends vscode.DebugSession {
176187
});
177188
})
178189
.catch((error: Error) => {
179-
this.sendEvent(new vscode.OutputEvent(error.message, 'stderr'));
190+
this.sendEvent(new vscode.OutputEvent(error.message));
180191
});
181192
} else {
182193
const script = childProcess.spawn(runtimeExecutable, [...runtimeArgs, args.program, ...programArgs], {cwd, env});
@@ -192,7 +203,7 @@ class PhpDebugSession extends vscode.DebugSession {
192203
this.sendEvent(new vscode.TerminatedEvent());
193204
});
194205
script.on('error', (error: Error) => {
195-
this.sendEvent(new vscode.OutputEvent(error.message, 'stderr'));
206+
this.sendEvent(new vscode.OutputEvent(error.message));
196207
});
197208
}
198209
}
@@ -295,23 +306,33 @@ class PhpDebugSession extends vscode.DebugSession {
295306
const log = `<- ${response.command}Response\n${util.inspect(response, {depth: null})}\n\n`;
296307
console[response.success ? 'log' : 'error'](log);
297308
if (this._args && this._args.log) {
298-
this.sendEvent(new vscode.OutputEvent(log, response.success ? 'stdout' : 'stderr'));
309+
this.sendEvent(new vscode.OutputEvent(log));
299310
}
300311
super.sendResponse(response);
301312
}
302313

314+
protected sendErrorResponse(response: VSCodeDebugProtocol.Response, error: Error, dest?: vscode.ErrorDestination): void;
315+
protected sendErrorResponse(response: VSCodeDebugProtocol.Response, codeOrMessage: number | VSCodeDebugProtocol.Message, format?: string, variables?: any, dest?: vscode.ErrorDestination): void;
316+
protected sendErrorResponse() {
317+
if (arguments[1] instanceof Error) {
318+
super.sendErrorResponse(arguments[0], arguments[1].code, arguments[1].message, arguments[2]);
319+
} else {
320+
super.sendErrorResponse(arguments[0], arguments[1], arguments[2], arguments[3], arguments[4]);
321+
}
322+
}
323+
303324
/** This is called for each source file that has breakpoints with all the breakpoints in that file and whenever these change. */
304325
protected setBreakPointsRequest(response: VSCodeDebugProtocol.SetBreakpointsResponse, args: VSCodeDebugProtocol.SetBreakpointsArguments) {
305326
const fileUri = this.convertClientPathToDebugger(args.source.path);
306327
const connections = Array.from(this._connections.values());
307328
let xdebugBreakpoints: Array<xdebug.ConditionalBreakpoint|xdebug.LineBreakpoint>;
308329
response.body = {breakpoints: []};
309330
// this is returned to VS Code
310-
let vscodeBreakpoints: vscode.Breakpoint[];
331+
let vscodeBreakpoints: VSCodeDebugProtocol.Breakpoint[];
311332
let breakpointsSetPromise: Promise<any>;
312333
if (connections.length === 0) {
313334
// if there are no connections yet, we cannot verify any breakpoint
314-
vscodeBreakpoints = args.breakpoints.map(breakpoint => new vscode.Breakpoint(false, breakpoint.line));
335+
vscodeBreakpoints = args.breakpoints.map(breakpoint => ({verified: false, line: breakpoint.line}));
315336
breakpointsSetPromise = Promise.resolve();
316337
} else {
317338
vscodeBreakpoints = [];
@@ -340,14 +361,13 @@ class PhpDebugSession extends vscode.DebugSession {
340361
.then(xdebugResponse => {
341362
// only capture each breakpoint once
342363
if (connectionIndex === 0) {
343-
vscodeBreakpoints.push(new vscode.Breakpoint(true, breakpoint.line));
364+
vscodeBreakpoints.push({verified: true, line: breakpoint.line, id: xdebugResponse.breakpointId});
344365
}
345366
})
346367
.catch(error => {
347368
// only capture each breakpoint once
348369
if (connectionIndex === 0) {
349-
console.error('breakpoint could not be set: ', error.message);
350-
vscodeBreakpoints.push(new vscode.Breakpoint(false, breakpoint.line));
370+
vscodeBreakpoints.push({verified: false, line: breakpoint.line, message: error.message});
351371
}
352372
})
353373
)))
@@ -359,7 +379,7 @@ class PhpDebugSession extends vscode.DebugSession {
359379
this.sendResponse(response);
360380
})
361381
.catch(error => {
362-
this.sendErrorResponse(response, error.code, error.message);
382+
this.sendErrorResponse(response, error);
363383
});
364384
}
365385

@@ -389,7 +409,7 @@ class PhpDebugSession extends vscode.DebugSession {
389409
)).then(() => {
390410
this.sendResponse(response);
391411
}).catch(error => {
392-
this.sendErrorResponse(response, error.code, error.message);
412+
this.sendErrorResponse(response, error);
393413
});
394414
}
395415

@@ -485,7 +505,7 @@ class PhpDebugSession extends vscode.DebugSession {
485505
this.sendResponse(response);
486506
})
487507
.catch(error => {
488-
this.sendErrorResponse(response, error.code, error.message);
508+
this.sendErrorResponse(response, error);
489509
});
490510
}
491511

@@ -535,7 +555,7 @@ class PhpDebugSession extends vscode.DebugSession {
535555
this.sendResponse(response);
536556
})
537557
.catch(error => {
538-
this.sendErrorResponse(response, error.code, error.message);
558+
this.sendErrorResponse(response, error);
539559
});
540560
}
541561
}
@@ -568,7 +588,7 @@ class PhpDebugSession extends vscode.DebugSession {
568588
const property = this._evalResultProperties.get(variablesReference);
569589
propertiesPromise = Promise.resolve(property.hasChildren ? property.children : []);
570590
} else {
571-
this.sendErrorResponse(response, 0, 'Unknown variable reference');
591+
this.sendErrorResponse(response, new Error('Unknown variable reference'));
572592
return;
573593
}
574594
propertiesPromise
@@ -597,61 +617,61 @@ class PhpDebugSession extends vscode.DebugSession {
597617
})
598618
.catch(error => {
599619
console.error(util.inspect(error));
600-
this.sendErrorResponse(response, error.code, error.message);
620+
this.sendErrorResponse(response, error);
601621
});
602622
}
603623
}
604624

605625
protected continueRequest(response: VSCodeDebugProtocol.ContinueResponse, args: VSCodeDebugProtocol.ContinueArguments): void {
606626
if (!args.threadId) {
607-
this.sendErrorResponse(response, 0, 'No active connection');
627+
this.sendErrorResponse(response, new Error('No active connection'));
608628
return;
609629
}
610630
const connection = this._connections.get(args.threadId);
611631
connection.sendRunCommand()
612632
.then(response => this._checkStatus(response))
613-
.catch(error => this.sendErrorResponse(response, error.code, error.message));
633+
.catch(error => this.sendErrorResponse(response, error));
614634
this.sendResponse(response);
615635
}
616636

617637
protected nextRequest(response: VSCodeDebugProtocol.NextResponse, args: VSCodeDebugProtocol.NextArguments): void {
618638
if (!args.threadId) {
619-
this.sendErrorResponse(response, 0, 'No active connection');
639+
this.sendErrorResponse(response, new Error('No active connection'));
620640
return;
621641
}
622642
const connection = this._connections.get(args.threadId);
623643
connection.sendStepOverCommand()
624644
.then(response => this._checkStatus(response))
625-
.catch(error => this.sendErrorResponse(response, error.code, error.message));
645+
.catch(error => this.sendErrorResponse(response, error));
626646
this.sendResponse(response);
627647
}
628648

629649
protected stepInRequest(response: VSCodeDebugProtocol.StepInResponse, args: VSCodeDebugProtocol.StepInArguments): void {
630650
if (!args.threadId) {
631-
this.sendErrorResponse(response, 0, 'No active connection');
651+
this.sendErrorResponse(response, new Error('No active connection'));
632652
return;
633653
}
634654
const connection = this._connections.get(args.threadId);
635655
connection.sendStepIntoCommand()
636656
.then(response => this._checkStatus(response))
637-
.catch(error => this.sendErrorResponse(response, error.code, error.message));
657+
.catch(error => this.sendErrorResponse(response, error));
638658
this.sendResponse(response);
639659
}
640660

641661
protected stepOutRequest(response: VSCodeDebugProtocol.StepOutResponse, args: VSCodeDebugProtocol.StepOutArguments): void {
642662
if (!args.threadId) {
643-
this.sendErrorResponse(response, 0, 'No active connection');
663+
this.sendErrorResponse(response, new Error('No active connection'));
644664
return;
645665
}
646666
const connection = this._connections.get(args.threadId);
647667
connection.sendStepOutCommand()
648668
.then(response => this._checkStatus(response))
649-
.catch(error => this.sendErrorResponse(response, error.code, error.message));
669+
.catch(error => this.sendErrorResponse(response, error));
650670
this.sendResponse(response);
651671
}
652672

653673
protected pauseRequest(response: VSCodeDebugProtocol.PauseResponse, args: VSCodeDebugProtocol.PauseArguments): void {
654-
this.sendErrorResponse(response, 0, 'Pausing the execution is not supported by XDebug');
674+
this.sendErrorResponse(response, new Error('Pausing the execution is not supported by XDebug'));
655675
}
656676

657677
protected disconnectRequest(response: VSCodeDebugProtocol.DisconnectResponse, args: VSCodeDebugProtocol.DisconnectArguments): void {
@@ -671,7 +691,7 @@ class PhpDebugSession extends vscode.DebugSession {
671691
this.sendResponse(response);
672692
});
673693
}).catch(error => {
674-
this.sendErrorResponse(response, error.code, error.message);
694+
this.sendErrorResponse(response, error);
675695
});
676696
}
677697

@@ -696,7 +716,7 @@ class PhpDebugSession extends vscode.DebugSession {
696716
this.sendResponse(response);
697717
})
698718
.catch(error => {
699-
this.sendErrorResponse(response, error.code, error.message);
719+
this.sendErrorResponse(response, error);
700720
});
701721
}
702722
}

‎src/typings/xmldom/xmldom.d.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
declare module 'xmldom' {
2+
interface DOMParserOptions {
3+
locator?: any;
4+
errorHandler?: ((level: string, msg: string) => any) | {
5+
warning?: (warning: any) => any;
6+
error?: (error: any) => any;
7+
fatalError?: (error: any) => any;
8+
};
9+
}
210
export class DOMParser {
11+
constructor(options: DOMParserOptions);
312
parseFromString(xml: string, mimeType: string): XMLDocument;
413
}
514
}

‎src/xdebugConnection.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,6 @@ export class Connection extends DbgpConnection {
525525
this._initPromise = new Promise<InitPacket>((resolve, reject) => {
526526
this._initPromiseResolveFn = resolve;
527527
});
528-
console.log('New XDebug Connection #' + this.id);
529528
}
530529

531530
/** Returns a promise that gets resolved once the init packet arrives */

0 commit comments

Comments
 (0)